c++ pointer comparer not compiling

Question

I'd like to sort an array of pointers, however VS won't compile, saying

'testClass::compareItems': non-standard syntax; use '&' to create a pointer to member

The comparer looks like this:

bool testClass::compareItems(ElementType *a, ElementType *b)
{       
    return elementToProfit[a] / a->w() > elementToProfit[b] / b->w();
}

while the array is just a normal array.

for (auto &knapsack : knapsacks)
{       
    std::sort(knapsack.second.begin(), knapsack.second.end(), compareItems);
}

I'm not quite sure what's going on. VS is also complaining that

'void std::sort(_RanIt,_RanIt)': expects 2 arguments - 3 provided

which I assume is because there's an issue with the comparer? This should be super easy... any help is greatly apprecitated, thanks!

what is `elementToProfit` ? and for your last point, I am not sure, but I think the comparator needs to be a free function not member function of some class — 463035818_is_not_an_ai, Oct 06 '15 at 18:08
...and why are you using a loop to sort the array? It should suffice to sort it once (and not once per element) — 463035818_is_not_an_ai, Oct 06 '15 at 18:09
I'm trying to sort many arrays, that's just a loop over the different arrays. I see where the problem is, it makes complete sense. "elementToProfit" is extra information outside the elemnts I want to sort, that I need for sorting though. I'm guessing that means I need to sort them "by hand", doesn't it, since I can't hand over that extra info? — master_axe, Oct 06 '15 at 18:28
I guess you can use a lambda to do that. Unfortunately I am not familiar enough with them to tell you how — 463035818_is_not_an_ai, Oct 06 '15 at 18:29
maybe you should consider explaining a bit more in detail what you want to do in the question to get better answers, because simply sorting the elements of the array by means of the values of the elements seems not to be enough — 463035818_is_not_an_ai, Oct 06 '15 at 18:30
.. or use a vector of pairs, i.e. `vector>` then you have access to both in the comparison function — 463035818_is_not_an_ai, Oct 06 '15 at 18:32
just for the sake of completeness I should mention that I recently heard a Scott Meyers talk where he strongly discourages to use pair, but thats a different topic — 463035818_is_not_an_ai, Oct 06 '15 at 18:33
here is an answer that explains quite in detail how to one vector to sort another one: http://stackoverflow.com/a/17074762/4117728 — 463035818_is_not_an_ai, Oct 06 '15 at 18:50

score 4 · Answer 1 · edited May 23 '17 at 12:01

The compare function can't be a non-static member of the class, since it isn't called on an instance of the class. It can be a static class function, a free-standing function, or a functor.

Judging by comments on the question, you may find a functor to be the best way forward. A functor is simply a class that implements operator() so that you can use an object of it with function call syntax. The benefit of this approach is that the object can contain additional members that you need to carry along for the comparison.

typedef std::unordered_map<ElementType*, double> ProfitType;

class functorClass
{
    ProfitType & elementToProfit;

public:
    functorClass(ProfitType & pt) : elementToProfit(pt) {}

    bool operator()(ElementType *a, ElementType *b)
    {       
        return elementToProfit[a] / a->w() > elementToProfit[b] / b->w();
    }
};

functorClass functor(elementToProfit);
for (auto &knapsack : knapsacks)
{       
    std::sort(knapsack.second.begin(), knapsack.second.end(), functor);
}

Marco A. · Answer 2 · 2015-10-06T18:21:38.250

A member function has an extra argument (the this pointer for the instance) therefore your signature isn't matching - the std::sort needs to call the function without an object reference.

The documentation also states that it should satisfy the requirements of a binary predicate.

This is a sample reproducer

struct ElementType {
  int w() {
    return 2;
  }
};

class testClass {
public:
  bool compareItems(ElementType *a, ElementType *b)
  {
    return a->w() < b->w();
  }

  void sort() {

    vector<ElementType*> vecOfPointers;
    std::sort(vecOfPointers.begin(), vecOfPointers.end(), compareItems);
  }

  vector<ElementType*> elementToProfit;
};

Example

You can fix your code by making your comparison function static (example).

Your second error is a direct consequence of the first one. Fix the first and you'll solve the second as well.

Thank you, that makes complete sense. I need the outside info to sort the elements, which can't be given to a comparer, I'm guessing? That value of an element is stored elsewhere, that's why. — master_axe, Oct 06 '15 at 18:29
@master_axe Use a functor or a lambda with some capture-by-reference for variables in scope. It's probably also more readable — Marco A., Oct 06 '15 at 18:30

score 0 · Answer 3 · edited Nov 11 '16 at 16:02

0

Two steps I found: (Assume knapsack.second is vector)

Define it as bool opeator()(...){...};
sort(knapsack.second.begin(), knapsack.second.end(),*this).

edited Nov 11 '16 at 16:02

Tunaki

132,869
46
340
423

answered Nov 11 '16 at 15:58

AXIS

1

c++ pointer comparer not compiling

3 Answers3