14

I declared a vector as follows: vector<unique_ptr<Worker>> Workers. Worker is a base class with a private field name and it has two derived classes: Builder and Driver.

I add to the Workers vector objects of Builder and Driver and then I want to sort the vector by name using #include <algorithm> like this:

sort(Workers.begin(), Workers.end(), cmp_by_name);

bool cmp_by_name(const Worker &a, const Worker &b)
{
    return a.getName() < b.getName();
}

But the VS compiler says:

Error 1 error C2664: 'bool (const Worker &,const Worker &)' : cannot convert argument 2 from 'std::unique_ptr>' to 'const Worker &' c:\program files (x86)\microsoft visual studio 12.0\vc\include\algorithm 3071 1 App

How can I fix this error?


Thanks to @NathanOliver, @Rabbid76 and this question, I edited my cmp_by_name into this form:

struct cmp_by_name
{
    inline bool operator()(const unique_ptr<Worker>& a, const unique_ptr<Worker>& b)
    {
        return a->getName() < b->getName();
    }
};

And I call the sort function like this:

sort(Workers.begin(), Workers.end(), cmp_by_name());
honk
  • 9,137
  • 11
  • 75
  • 83
Da Artagnan
  • 1,109
  • 3
  • 17
  • 26
  • 1
    Signature of the predicate is wrong. – Lingxi Jan 20 '16 at 17:08
  • You should move the part with your solution into a new answer. This way, your post would better fit Stack Overflow's Q&A format and people could provide upvotes for you solution. – honk Dec 11 '18 at 15:54

3 Answers3

22

The comparison function which std::sort uses needs to be in the form of:

bool cmp(const Type1 &a, const Type2 &b);

Here the types Type1 and Type2 must be such that the iterator can be dereferenced and then implicitly converted to both of them.

In your case dereferencing Workers.begin() gives you a unique_ptr<Worker> not a Worker. You will need to change your comparison function to take a const unique_ptr<Worker>&.

In this case it would wind up looking like:

bool cmp_by_name(const std::unique_ptr<Worker>& a, const std::unique_ptr<Worker>& b)
{
    return a->getName() < b->getName();
}
honk
  • 9,137
  • 11
  • 75
  • 83
NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • Can you tell me how should I call function sort(Workers.begin(), Workers.end(), cmp_by_name); in that case? – Da Artagnan Jan 20 '16 at 17:20
  • @DaArtagnan I am not sure what you mean. If you chage `cmp_by_name` to the way I have it in the answer then it should work. – NathanOliver Jan 20 '16 at 17:21
6

The datatype of your std::vector<std::unique_ptr<Worker>> is std::unique_ptr<Worker>, so your comparison function has to look like this:

bool cmp_by_name(const std::unique_ptr<Worker> &a, const std::unique_ptr<Worker> &b)
{
    return a->getName() < b->getName();
}

The comparison function expects to arguments so that an object of the std::vector can convert to them.

Rabbid76
  • 202,892
  • 27
  • 131
  • 174
2

Since C++11, you can also use a lambda expression instead of defining a comparison function:

int main()
{
    using workers_t = std::unique_ptr<Worker>;
    std::vector<workers_t> Workers;
    Workers.emplace_back(std::make_unique<Worker>(Worker("Paul")));
    Workers.emplace_back(std::make_unique<Worker>(Worker("Anna")));
    Workers.emplace_back(std::make_unique<Worker>(Worker("John")));

    std::sort(std::begin(Workers), std::end(Workers), [](const workers_t& a, const workers_t& b) {
        return a->getName() < b->getName();
    });

    for (auto const &worker : Workers)
        std::cout << worker->getName() << std::endl;

    return 0;
}

Note: This example directly uses Worker objects for the sake of clarity, but it should work for your derived Builder and Driver objects as well.

Code on Ideone

honk
  • 9,137
  • 11
  • 75
  • 83