Having trouble 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 do my sort, the LogEvent objects in currentItems are not sorted correctly. I am pretty sure my operator overloading is working.

When I do stuff like this:

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

it will output the right value.

So what am I doing wrong and how do I correct it?

Answer 1

qSort is sorting the pointers, not the objects pointed to by those pointers. If you want to sort LogEvents with qSort, you will have to store them by value and not by reference (and also have comparison operators that take a reference, qSort won't find your comparison-to-pointer functions), or pass a third argument with a function you define.

It might bear explaining why this is so with 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 a single complete answer to accept, I'm going to rip off some good bits from the other answers here.

First, define a standard syntax less than 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 dereference-and-compare from leemes' answer:

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

to sort your list like so:

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

For completeness, it would be good form to define standard syntax comparison operators for all of your comparisons. Whether you keep your non-standard comparison operators is up to you.

Answer 2

You could define a comparison function taking two (generic) pointers:

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

And then call

void qSort ( RandomAccessIterator begin, RandomAccessIterator end, LessThan lessThan )

like this:

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

You then can use this method for other types too without the need to define multiple functions.