views:

835

answers:

3

Hi all! The title is the main question. The exact scenario (I am 'using namespace std;'):

void SubstringMiner::sortByOccurrence(list<Substring *> & substring_list) {
   list::sort(substring_list.begin(), substring_list.end(), Substring::OccurrenceComparator);
}

This is the comparator definition:

class Substring {
    // ...
    class OccurrenceComparator {
        public:
            bool operator() (Substring * a, Substring *b);
    }
};

Implementation of the comparator is intuitive and trivial. I am also using a very similar comparator in a std::set and it works fine. When I add the sortByOccurrence() funcion it gives me the error in the title.

What should I do?

EDIT: I'm now trying to pass Substring::OccurrenceComparator() as the comparator, and am getting the following error:

g++ -Wall -g -c substring_miner.cpp -o obj/subtring_miner.o
substring_miner.cpp: In function ‘void SubstringMiner::sortByOccurrence(std::list<Substring*, std::allocator<Substring*> >&)’:
substring_miner.cpp:113: error: no matching function for call to ‘std::list<Substring*, std::allocator<Substring*> >::sort(std::_List_iterator<Substring*>, std::_List_iterator<Substring*>, Substring::OccurrenceComparator)’
/usr/include/c++/4.3/bits/list.tcc:303: note: candidates are: void std::list<_Tp, _Alloc>::sort() [with _Tp = Substring*, _Alloc = std::allocator<Substring*>]
make: *** [substring_miner] Error 1

My code line is now:

list<Substring *>::sort(substring_list.begin(), substring_list.end(), Substring::OccurrenceComparator());

I can't remove the template or it gives me an error saying that template parameters were wrong.

+4  A: 

You're passing a class as an argument to a function. You cannot do that - you have to create an instance of the class, and pass that:

substring_list.sort(Substring::OccurrenceComparator());

Note the extra parentheses after OccurenceComparator above that create a temporary object of the class using default constructor.

Another mistake is that you're calling list::sort as a static function on class std::list. It's not static, so you need to call it as a member function on substring_list.

Pavel Minaev
Please check question edit.
Rafael Almeida
+3  A: 

The original problem was already solved by Pavel Minaev above.
But some extra notes.

The operator() should probably be const (as well as the parameters).
For simple classes like this it is easier to just make them structures.

struct OccurrenceComparator
{
    bool operator() (Substring const* a, Substring const* b)  const;
};

Note the comparison must provide a strict weak ordering:

template<class BinaryPredicate>
void sort(BinaryPredicate comp);

Comp must be a comparison function that induces a strict weak ordering (as defined in the LessThan Comparable requirements on objects of type T. This function sorts the list *this according to Comp. The sort is stable, that is, the relative order of equivalent elements is preserved. All iterators remain valid and continue to point to the same elements. [6] The number of comparisons is approximately N log N, where N is the list's size.

Martin York
Thanks for the tips. The weak ordering predicate was already implemented.
Rafael Almeida
+4  A: 

list member sort is a non-static function so must be called on a list instance.

substring_list.sort( Substring::OccurrenceComparator() );

Edit: You can't use the free function std::sort as it requires random access iterators which list iterators are not.

Charles Bailey
It actually solves it. Since the specification says the sort is stable, I'm sticking with it.
Rafael Almeida