Vector sorting using Strategy pattern

[Mercurialol]

// main.cpp
Vector<Student*> students;
AbstractStrategy* strategy;

strategy= new NameAscendingSortStrategy;

//push_back a couple of students into vector
printVector(students);

strategy->sort(students);

printVector(students);

Now... What I had in mind is this:

//AbstractStrategy.h
class AbstractStrategy{
public:
       void sort(vector<Student*>& students){
            std::sort(students.begin(), students.end(), comparator); 
       }
       virtual bool comparator(Student*,Student*)= 0;
};

// NameAscendingSortStrategy.h
class NameAscendingSortStrategy : public AbstractStrategy{
public:
       bool comparator(Student* first, Student* second){
            return first->getName()< second->getName();
       }
};

but apparently member functions take extra this argument and cannot be passed as that to std::sort.

The other solution I had in mind was just separating comparators as structures with overloaded operator().

So instead of using concrete strategies to change the comparator in the superclass, I'd

// NameAscendingSortComparator.h
struct NameAscendingSortComparator{
       bool operator()(Student* first, Student* second){
            return first->getName()< second->getName();
       }
};

and make a call in main:

std::sort(students.begin(), students.end(), NameAscendingSortComparator());

And it works, just that I must write the std::sort each time. I tried making a function in main that takes a vector as an argument and a pointer to function(comparator) but compailer says it can't convert second argument to bool(*)(Student*,Student*).

Thoughts?

[jsmith]

Since comparator() needs to access nothing instead AbstractStrategy, it can simply be declared a static method (though you have to remove the virtual-ness, which makes your AbstractStrategy concrete). This eliminates the "this" pointer problem.

[Mercurialol]

I am aware of that static functions are treated like non-member functions, but I can't override them then. I declared it virtual so I can get the concrete comparator ( NameAscending, NameDescending, SurnameAscending... ) via polymorphism.

[b2ee]

Couldn't understand "but apparently member functions take extra this argument and cannot be passed as that to std::sort. " Do you mean you can't call member function in sort()?

[Mercurialol]

Exactly that.

[jsmith]

Well, you can adapt it using std::mem_fun or boost::function, which would solve OP's original problem.

However I'm questioning the need for polymorphism at all. Making the comparator a virtual function will considerably affect the performance of the sort for large containers.

[Mercurialol]

I agree. How would you do it ?

[jsmith]

Well, so I don't know the entire scope of what you are trying to do, so I can only answer based on what you've posted above.

All your Strategy objects are nothing more than comparison functions with a single sort() method in the base that allows you to write the call to std::sort only once.

Personally, I'd write the std::sort line wherever I need it, because it is more work (typing) for the programmer to instantiate a concrete strategy and then call the sort method on it.

But, one alternative is to make a template function:

namespace my_ns
{
    template< typename Container, typename Comparator >
    void sort( Container& container, Comparator comp )
    { std::sort( container.begin(), container.end(), comp ) };
}

Another method is to use std::mem_fun (http://www.cplusplus.com/reference/std/functional/mem_fun/). However, I prefer boost::bind and boost::function as they are easier to use than <functional>:

//AbstractStrategy.h
class AbstractStrategy{
public:
       void sort(vector<Student*>& students){
            std::sort(students.begin(), students.end(), boost::bind( &AbstractStrategy::comparator, this, _1, _2 ) ); 
       }
       virtual bool comparator(Student*,Student*) const = 0;
};

However, I'd go with the first approach for reasons of performance.

[Mercurialol]

Thanks alot.