passing a class method as opposed to a function in std::sort

Question

hi,

Within a class, I am trying to sort a vector, by passing a method of the same class. But it gives errors at the time of compilation. Can anyone tell what the problem is? Thank you!

it gives the following error: argument of type bool (Sorter::)(D&, D&)' does not matchbool (Sorter::*)(D&, D&)'

I have also tried using sortBynumber(D const& d1, D const& d2)

#include<vector>
#include<stdio.h>
#include<iostream>
#include<algorithm>

class D {
      public:                   
             int getNumber();            
             D(int val);
             ~D(){};
      private:
              int num;
};

D::D(int val){
         num = val;
         };

int D::getNumber(){
    return num;
};


class Sorter {
      public:                   
             void doSorting();  
             bool sortByNumber(D& d1, D& d2);
             std::vector<D> vec_D;          
             Sorter();
             ~Sorter(){};
      private:
              int num;
};

Sorter::Sorter(){                 
        int i;
        for ( i = 0; i < 10; i++){
            vec_D.push_back(D(i));
           }
         };

bool Sorter::sortByNumber(D& d1, D& d2){
     return d1.getNumber() < d2.getNumber();
     };

void Sorter::doSorting(){
     std::sort(vec_D.begin(), vec_D.end(), this->sortByNumber);
     };




int main(){    
    Sorter s;
    s.doSorting();

    std::cout << "\nPress RETURN to continue...";
    std::cin.get();

    return 0;
}

Answer 1

Make Sorter::sortByNumber static. Since it doesn't reference any object members, you won't need to change anything else.

class Sorter {
public:                   
    static bool sortByNumber(const D& d1, const D& d2);
    ...
};

// Note out-of-class definition does not repeat static
bool Sorter::sortByNumber(const D& d1, const D& d2)
{
    ...
}

You should also use const references as sortByNumber should not be modifying the objects.

Answer 2

I don't see any reason to make sortByNumber as a member function of class Sorter. You can do the sorting much more easily avoiding all the ugly bind code if you make it a free function. Also, you should use const wherever it is applicable in the code. Following is the example of doing it using free function:

First change the int getNumber() to const function as int getNumber() const;

Then write your free function sortByNumber again taking parameters by const reference. bool sortByNumber(const D& d1, const D& d2);

You can call sort as : std::sort(vec_D.begin(), vec_D.end(), sortByNumber);

Answer 3

I see no reason for sortByNumber() to be a member function. When it's a member function it gains access to things it doesn't need (and therefore shouldn't have access to). Either extract the method and refactor it into a function object:

struct sortByNumber {
    bool operator()(const D& d1, const D& d2) const {
        return d1.getNumber() < d2.getNumber();
    }
};

or make it a free function. Given the choice you should prefer a function object, because that makes it possible for the compiler to inline the code if it so chooses. Then, you can sort like so:

std::sort(vec_D.begin(), vec_D.end(), sortByNumber());

That said, you can get the code to compile as is like so, with boost::bind():

std::sort(vec_D.begin(), vec_D.end(),
          boost::bind(&Sorter::sortByNumber, this, _1, _2));

You will need the boost libraries for that to work, and you will need to #include <boost/bind.hpp>.

Answer 4

Unless you have a really good reason to do otherwise, just define operator< for the type of items you're sorting, and be done with it:

class D { 
    int val;
public:
    D(int init) : val(init) {}
    bool operator<(D const &other) { return val < other.val; }
};

class sorter { 
    std::vector<D> vec_D;
public:
    void doSorting() { std::sort(vec_d.begin(), vec_D.end()); }
};

The way you're writing your sorter class depends on knowing a lot about the internals of the D class, to the point that they're practically a single class (e.g., it looks like neither can do much of anything without the other).

At a guess, your sorter may be a somewhat stripped-down version of your real code. The SortByNumber makes it sound like the original code might support a number of different kinds of keys, something like:

class D { 
    std::string name;
    int height;
    int weight;
// ...
};

and you'd want to be able to sort D objects by name, height, or weight. In a case like that, the comparisons are really still related to the D class, so I'd probably put them into a common namespace:

namespace D { 
class D { 
    std::string name;
    int height;
    int weight;
public:
    friend class byWeight;
    friend class byHeight;
    friend class byName;
    // ...
};

struct byWeight { 
   bool operator()(D const &a, D const &b) { 
       return a.weight < b.weight;
   }
};

struct byHeight {
    bool operator()(D const &a, D const &b) { 
        return a.height < b.height;
    }
};

struct byName { 
    bool operator()(D const &a, D const &b) { 
        return a.name < b.name;
    }
};
}

Then sorting would look something like:

std::vector<D::D> vec_D;

// sort by height:
std::sort(vec_D.begin(), vec_D.end(), D::byHeight());

// sort by weight:
std::sort(vec_D.begin(), vec_D.end(), D::byWeight());

// sort by name:
std::sort(vec_D.begin(), vec_D.end(), D::byName());

Note that this does not use free functions. For this kind of purpose, a functor is generally preferable. I've also used a namespace to show the association between the object being sorted and the different ways of sorting it. You could make them nested classes instead, but I'd generally prefer the common namespace (keep coupling as loose as reasonable).

In any case, I would not give access to the raw data (even read-only access) via the object's public interface if it could be avoided (and in this case, it can be).