Problems Using qSort

Question

I have this code:

QVector<LogEvent *> currentItems; //add a bunch of LogEvent objects to currentItems qSort(currentItems.begin(), currentItems.end()); 

This is my LogEvent class:

LogEvent.h:

 //LogEvent.h class LogEvent : public QTreeWidgetItem { public: LogEvent(); LogEvent(QDateTime, LogEvent *parent = 0); ~LogEvent(); bool operator<(const LogEvent *); bool operator>(const LogEvent *); bool operator<=(const LogEvent *); bool operator>=(const LogEvent *); bool operator==(const LogEvent *); private: QDateTime timestamp; }; 

LogEvent.cpp:

 //LogEvent.cpp LogEvent::LogEvent() { } LogEvent::LogEvent(QDateTime timestamp, LogEvent *parent) : QTreeWidgetItem(parent) { this->timestamp = timestamp; } bool LogEvent::operator<(const LogEvent * event) { return (this->timestamp < event->timestamp); } bool LogEvent::operator>(const LogEvent * event) { return (this->timestamp > event->timestamp); } bool LogEvent::operator<=(const LogEvent * event) { return (this->timestamp <= event->timestamp); } bool LogEvent::operator>=(const LogEvent * event) { return (this->timestamp >= event->timestamp); } bool LogEvent::operator==(const LogEvent * event) { return (this->timestamp == event->timestamp); } 

After I make my appearance, the LogEvent objects in currentItems are not sorted correctly. I am sure my operator overload is working.

When I do like this:

 std::cout << currentItems[0]<=currentItems[1]?"T":"F"; 

it gives the correct value.

So what am I doing wrong and how to fix it?

Answer 1

qSort sorts the pointers, not the objects that these pointers point to. If you want to sort LogEvents with qSort, you will have to store them by value, not by reference (and also have comparison operators that take a link, qSort will not find your comparison functions with a pointer) or pass a third argument with the function you define.

This may explain why this is so with the examples.

 LogEvent event1, event2; LogEvent *eventptr1=&event1,*eventptr2=&event2; event1<event2; // Operator not defined in your code event1<eventptr2; // This will call the operator you have defined eventptr1<eventptr2; // This will compare the pointers themselves, not the LogEvents. The pointers are not dereferenced here. 

ETA: In the interest of having one complete answer to accept, I am going to rip off some good bits from the other answers here.

First, define the standard syntax less than the operator:

 class LogEvent : public QTreeWidgetItem { public: // ... bool operator<(const LogEvent *); // Non-standard, possibly reasonable for use in your own code. bool operator<(const LogEvent &); // Standard, will be used by most template algorithms. // ... } 

LogEvent.cpp

 bool LogEvent::operator<(const LogEvent &event) {return timestamp<event.timestamp;} 

Once this is done, you can use this template to dereference and compare leemes answers:

 template<class T> bool dereferencedLessThan(T * o1, T * o2) { return *o1 < *o2; } 

to sort your list as follows:

 QVector<LogEvent *> currentItems; //add a bunch of LogEvent objects to currentItems qSort(list.begin(), list.end(), dereferencedLessThan<LogEvent>); 

For completeness, it would be a good form to define standard syntax comparison operators for all your comparisons. Regardless of whether you support your custom comparison operators, you yourself.

Answer 2

You can define a comparison function with two (universal) pointers:

 template<class T> bool dereferencedLessThan(T * o1, T * o2) { return *o1 < *o2; } 

And then call

void qSort (beginning of RandomAccessIterator, end of RandomAccessIterator, LessThan lessThan)

like this:

 qSort(list.begin(), list.end(), dereferencedLessThan<LogEvent>); 

Then you can use this method for other types too, without having to define several functions.

Answer 3

Should comparison operators use const references rather than pointers?

In the last fragment, you are comparing addresses, not values.

Answer 4

The qSort function requires that the element type (in the above example, LogEvent) execute the <() operator. For instance:

 bool LogEvent::operator<(const LogEvent& event) { return timestamp < event.timestamp; }