40

trying to compile the following code I get this compile error, what can I do?


ISO C++ forbids taking the address of an unqualified or parenthesized non-static member function to form a pointer to member function.

class MyClass {
   int * arr;
   // other member variables
   MyClass() { arr = new int[someSize]; }

   doCompare( const int & i1, const int & i2 ) { // use some member variables } 

   doSort() { std::sort(arr,arr+someSize, &doCompare); }

}; 
1

10 Answers 10

32

doCompare must be static. If doCompare needs data from MyClass you could turn MyClass into a comparison functor by changing:

doCompare( const int & i1, const int & i2 ) { // use some member variables } 

into

bool operator () ( const int & i1, const int & i2 ) { // use some member variables } 

and calling:

doSort() { std::sort(arr, arr+someSize, *this); }

Also, isn't doSort missing a return value?

I think it should be possible to use std::mem_fun and some sort of binding to turn the member function into a free function, but the exact syntax evades me at the moment.

EDIT: Doh, std::sort takes the function by value which may be a problem. To get around this wrap the function inside the class:

class MyClass {
    struct Less {
        Less(const MyClass& c) : myClass(c) {}
        bool operator () ( const int & i1, const int & i2 ) {// use 'myClass'} 
        MyClass& myClass;
    };
    doSort() { std::sort(arr, arr+someSize, Less(*this)); }
}
Sign up to request clarification or add additional context in comments.

1 Comment

There is still a problem in this solution. STL sort called the distructor of the object that is passed to it as comparator, this would ruin my program !!!
15

As Andreas Brinck says, doCompare must be static (+1). If you HAVE TO have a state in your comparator function (using the other members of the class) then you'd better use a functor instead of a function (and that will be faster):

class MyClass{

   // ...
   struct doCompare
   { 
       doCompare( const MyClass& info ) : m_info(info) { } // only if you really need the object state
       const MyClass& m_info;

       bool operator()( const int & i1, const int & i2  )
       { 
            // comparison code using m_info
       }
   };

    doSort() 
    { std::sort( arr, arr+someSize, doCompare(*this) ); }
};

Using a functor is always better, just longer to type (that can be unconvenient but oh well...)

I think you can also use std::bind with the member function but I'm not sure how and that wouldn't be easy to read anyway.

UPDATE 2014: Today we have access to c++11 compilers so you could use a lambda instead, the code would be shorter but have the exact same semantic.

2 Comments

Finally I found a sensible explanation for how to do this..! Thanks.
Very nice - and this can easily be adapted to a generic solution which takes a class ptr + method ptr.
12

The solution proposed by Rob is now valid C++11 (no need for Boost):

void doSort()
{
  using namespace std::placeholders;
  std::sort(arr, arr+someSize, std::bind(&MyClass::doCompare, this, _1, _2));
}

Indeed, as mentioned by Klaim, lambdas are an option, a bit more verbose (you have to "repeat" that the arguments are ints):

void doSort()
{
  std::sort(arr, arr+someSize, [this](int l, int r) {return doCompare(l, r); });
}

C++14 supports auto here:

void doSort()
{
  std::sort(arr, arr+someSize, [this](auto l, auto r) {return doCompare(l, r); });
}

but still, you declared that arguments are passed by copy.

Then the question is "which one is the most efficient". That question was treated by Travis Gockel: Lambda vs Bind. His benchmark program gives on my computer (OS X i7)

                        Clang 3.5    GCC 4.9
   lambda                    1001        7000
   bind                3716166405  2530142000
   bound lambda        2438421993  1700834000
   boost bind          2925777511  2529615000
   boost bound lambda  2420710412  1683458000

where lambda is a lambda used directly, and lambda bound is a lambda stored in a std::function.

So it appears that lambdas are a better option, which is not too much of a surprise since the compiler is provided with higher level information from which it can make profit.

Comments

5

You can use boost::bind:

void doSort() {
  std::sort(arr,arr+someSize, boost::bind(&MyClass::doCompare, this, _1, _2));
}

Comments

2

There is a way to do what you want, but you need to use a small adaptor. As the STL doesn't write it for you, can can write it yourself:

template <class Base, class T>
struct adaptor_t
{
  typedef bool (Base::*method_t)(const T& t1, const T& t2));
  adaptor_t(Base* b, method_t m)
    : base(b), method(m)
  {}
  adaptor_t(const adaptor_t& copy) : base(copy.base), method(copy.method) {}
  bool operator()(const T& t1, const T& t2) const {
    return (base->*method)(t1, t2);
  }
  Base *base;
  method_t method;
}
template <class Base, class T>
adaptor_t<Base,T> adapt_method(Base* b, typename adaptor_t<Base,T>::method_t m)
{  return adaptor_t<Base,T>(b,m); }

Then, you can use it:

doSort() { std::sort(arr,arr+someSize, adapt_method(this, &doCompare)); }

Comments

1

The third argument in the calling of std::sort() is not compatible to the function pointer needed by std::sort(). See my answer to another question for a detailed explanation for why a member function signature is different from a regular function signature.

Comments

1

just make your helper function, static which you are going to pass inside the sort function.

for e.g

struct Item
{
int val;
int id;
};

//Compare function for our Item struct
static bool compare(Item a, Item b)
{
return b.val>a.val;
}

Now you can pass this inside your sort function

Comments

0

A very simple way to effectively use a member function is to use operator<. That is, if you have a function called compare, you can call it from operator<. Here is a working example:

class Qaz
{
public:
Qaz(int aX): x(aX) { }

bool operator<(const Qaz& aOther) const
    {
    return compare(*this,aOther);
    }

static bool compare(const Qaz& aP,const Qaz& aQ)
    {
    return aP.x < aQ.x;
    }

int x;
};

Then you don't even need to give the function name to std::sort:

std::vector<Qaz> q;
q.emplace_back(8);
q.emplace_back(1);
q.emplace_back(4);
q.emplace_back(7);
q.emplace_back(6);
q.emplace_back(0);
q.emplace_back(3);
std::sort(q.begin(),q.end());

Comments

0

Updating Graham Asher answer, as you don't need the compare but can use the less operator directly.

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

class Qaz {
public:
    Qaz(int aX): x(aX) { }

    bool operator<(const Qaz& aOther) const {
       return x < aOther.x;
    }

int x;
};

int main() {
    std::vector<Qaz> q;
    q.emplace_back(8);
    q.emplace_back(1);
    q.emplace_back(4);
    q.emplace_back(7);
    q.emplace_back(6);
    q.emplace_back(0);
    q.emplace_back(3);
    std::sort(q.begin(),q.end());
    for (auto& num : q)
        std::cout << num.x << "\n";

    char c;
    std::cin >> c;
    return 0;
}

Comments

0

can some one give an example where comparison function is to be used for set. for example

#include <map>
#include <set>
#include <string>
#include <vector>
#include <algorithm>

using namespace std;

struct MyStruct {
    unsigned p;
    unsigned t;
};


class MyClass {
public:
    void call() {
        ob["o1_10"] ={10, 1};
        mbp[ob["o1_10"].p].insert("o1_10");

        ob["o2_20_2"] ={20, 2};
        mbp[ob["o2_20"].p].insert("o2_20");

        ob["o3_30"] ={30, 3};
        mbp[ob["o3_30"].p].insert("o3_30");

        ob["o4_4_4"] ={4, 4};
        mbp[ob["o4_4"].p].insert("o4_4");

        ob["o5_10"] ={10, 4};
        mbp[ob["o5_10"].p].insert("o5_10");
    }
    
    
private:
    map<unsigned,set<string, compare>> mbp;
    // Question: how to define compare using struct, operator(), statice metthod or external method so that
    //           compare fetches ob[ol] and ob[0l2] and decide based on some rules
    //           so, comparions is not based on ol and o2 and it is based on some p and t in ob[o1] and ob[02]
    // static bool compare2(string& o1, string& o2) {
    //     // TODO: iS it possible to use this as customed comparison for set in mbp?
    //     // if (this->ob[ol].p > ob[o2].p)   return true;
    //     // if (ob[ol].p == ob[o2].p && ob[ol].t < ob[o2].t) return true;

    //     return false;

    // }
    
    map<string, MyStruct> ob;
};


int main() {
    MyClass my_class;
    my_class.call();

   
    
    return 0;
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.