How can I sort an array using a custom comparator?

Question

When I call my build function from my code below, I get the following compile errors:

error C3867: 'SuffixArray::cmp': function call missing argument list; use '&SuffixArray::cmp' to create a pointer to member
error C2780: 'void std::sort(_RanIt,_RanIt)' : expects 2 arguments - 3 provided

This is my code:

class A{
private:
    std::string text;
    std::vector<int> array;

    bool cmp(int a, int b) {
        return std::lexicographical_compare(text.begin() + a, text.end(),
                                            text.begin() + b, text.end());
    }

    void build(const std::string &str, std::vector<int> &vec) {

        // Some vector values actions.

        std::sort(vec.begin(), vec.end(), cmp);
    }
};

What's wrong here? I'm using the Visual C++ compiler.

score 4 · Accepted Answer · answered Dec 07 '13 at 23:32

4

Your comparison function A::cmp is a non-static member of A. Thus, it takes three arguments: in addition to the two arguments explicitly declared, it also takes a pointer to A to become the implicitly available this. It also has a different type than normal function pointers: bool (A::)(int, int) which decays into bool (A::*)(int, int) when being passed by value.

You could std::bind() your function to a suitable object, however:

std::sort(vec.begin(), vec.end(),
          std::bind(&A::cmp, this, std::placeholders::_1, std::placeholders::_2));

answered Dec 07 '13 at 23:32

Dietmar Kühl

150,225
13
225
380

I was going to suggest him to make it a static function... your advice is a lot more complete. – Jekyll Dec 07 '13 at 23:35
@Jekyll: Suggesting a `static` member was my first idea, too, but the code actually uses `A::text` in the `A::cmp()` function, i.e., the object is actually needed. That is, making the function `static` wouldn't solve the problem! – Dietmar Kühl Dec 07 '13 at 23:37
since the bind function was only introduced in c++11 I wonder how could you deal with it before the new standard?(I don't really need it, just curious) – Dmitry S. Dec 07 '13 at 23:43
@SAD: well, you could use [Boost](http://www.boost.org/)'s `bind()` or write a custom function object class, e.g.: `struct cmp { std::string const* name; cmp(std::string const* name): name(name) {} bool operator()(int a, int a) const { return std::lexicographical_compare(...); } };` – Dietmar Kühl Dec 07 '13 at 23:46

score 0 · Answer 2 · answered Jul 06 '18 at 12:24

The answer by @DietmarKühl explains perfectly, why your code does not compile. But since C++11, you can also use a lambda expression instead of defining a comparison function or making it static:

#include <vector>
#include <algorithm>

class A{
private:
    std::string text;
    std::vector<int> array;

    void build(const std::string &str, std::vector<int> &vec) {

        // Some vector values actions.

        std::sort(vec.begin(), vec.end(), [this](int a, int b) {
            return std::lexicographical_compare(text.begin() + a, text.end(),
                                                text.begin() + b, text.end());
        });
    }
};

I order to be able to access the class member text in the lambda expression, I captured the this pointer. However, if you want an alternative to capturing this, please take a look at this answer.

How can I sort an array using a custom comparator?

2 Answers2