37

I have something like this:

class Bar
      {
      public:
        pair<string,string> one;
        std::vector<string> cars;
        Bar(string one, string two, string car);
      };

class Car
      {
      public:
        string rz;
        Bar* owner;
        Car(string car, Bar* p);
      };

class Foo
       {
         public:
          Foo  ( void );
          ~Foo  ( void );
          int Count ( const string & one, const string &  two) const;
          int comparator (const Bar & first, const Bar & second) const;            
            std::vector<Bar> bars;
       };

int Foo::comparator(const Bar & first, const Bar & second) const{
  return first.name < second.name;
}

int Foo::Count  ( const string & one, const string & two ) const{
  int result=0;
  Bar mybar = Bar( one, two, "" );
  std::vector<Bar>::iterator ToFind = lower_bound(bars.begin(), bars.end(), mybar, comparator);
  if (ToFind != bars.end() && ToFind->one == mybar.one ){
    result = ...
  }
  return result;
}

The method Foo::Count should use std::lower_bound() to find element in vector<Bar> according to pair of two strings. Now the part which doesn't work. To lower_bound() I'm providing method comparator(). I thought it was okay, but g++ says:

c.cpp: In member function ‘int Foo::Count(const string&, const string&) const’:
c.cpp:42:94: error: invalid use of non-static member function
std::vector<Bar>::iterator ToFind = lower_bound(bars.begin(), bars.end(), mybar, comparator);

And the method Count() must stay const...

I'm quite new to C++ because I'm forced to learn it.

Any ideas?

Null
  • 1,950
  • 9
  • 30
  • 33
Nash
  • 493
  • 1
  • 4
  • 7

3 Answers3

32

The simplest fix is to make the comparator function be static:

static int comparator (const Bar & first, const Bar & second);
^^^^^^

When invoking it in Count, its name will be Foo::comparator.

The way you have it now, it does not make sense to be a non-static member function because it does not use any member variables of Foo.

Another option is to make it a non-member function, especially if it makes sense that this comparator might be used by other code besides just Foo.

M.M
  • 138,810
  • 21
  • 208
  • 365
20

You must make Foo::comparator static or wrap it in a std::mem_fun class object. This is because lower_bounds() expects the comparer to be a class of object that has a call operator, like a function pointer or a functor object. Also, if you are using C++11 or later, you can also do as dwcanillas suggests and use a lambda function. C++11 also has std::bind too.

Examples:

// Binding:
std::lower_bounds(first, last, value, std::bind(&Foo::comparitor, this, _1, _2));
// Lambda:
std::lower_bounds(first, last, value, [](const Bar & first, const Bar & second) { return ...; });
Kenn Sebesta
  • 7,485
  • 1
  • 19
  • 21
Matthew
  • 771
  • 6
  • 15
  • 1
    You're welcome. I would make the case that the comparison function be static though since it does not access the class. – Matthew Mar 26 '15 at 19:44
7

You shall pass a this pointer to tell the function which object to work on because it relies on that as opposed to a static member function.

edmz
  • 8,220
  • 2
  • 26
  • 45