views:

200

answers:

3

Hello,

In my homework, I have to design a class Message; among other attributes, it has attribute "priority" (main goal is to implement priority queue).

As in container I must check if one object is greater than other, I have overloaded operator '>'. Now, I have a few general questions about it...

Question one:

If I overload operator '>', should I overload operator '<' for arguments (const Message&, const Message&)?

My opinion is that overloading both > and < and using it in code will generate an error:

if(message1 > message2)
   { ... }

(Does the following code calls operator > for message1 object, or operator < message2 object?)

But, what if I use operator like this:

if(message1 < message2)
   { ... }

? :confused:

operator> is declared as friend function:

friend bool operator>(const Message& m1, const Message& m2)

Does it need to be declared as member function?

Thank you.

+6  A: 

If I overload operator '>', should I overload operator '<' for argumenst (const Message&, const Message&)?

Yes. In fact, it’s convention in most code to prefer the usage of < over > (don’t ask me why, probably historical). But more generally, always overload the complete set of related operators; in your case, this would probably also be ==, !=, <= and >=.

(Does the following code calls operator > for message1 object, or operator < message2 object?)

It always calls what it finds in the code. For the C++ compiler, there’s absolutely no connection between > and <. For us, they look similar but the compiler sees two completely different, unrelated symbols. So there’s no ambiguity: the compiler calls what it sees.

Does it need to be declared as member function?

No. In fact, it’s best not declared as a member function. Declaring it as a member function means that the first argument (i.e. the left-hand side of the expression) must really be a Message object, rather than an object that is implicitly convertible to a Message.

To understand this, consider the following case:

struct RealFraction {
    RealFraction(int x) { this.num = x; this.den = 1; }
    RealFraction(int num, int den) { normalize(num, den); }
    // Rest of code omitted.

    bool operator <(RealFraction const& rhs) {
        return num * rhs.den < den * rhs.num;
    }
};

Now you can write the following comparison:

int x = 1;
RealFraction y = 2;
if (y < x) …

but you cannot write the following:

if (x < y) …

although there exists an implicit conversion from int to RealFraction (using the first constructor).

If, on the other hand, you had used a non-member function to implement the operator, both comparisons would work because C++ would know to call an implicit constructor on the first argument.

Konrad Rudolph
Thank you, I read somewhere that if I overload one relation operation, it seems rational to overload all of them.
Burgos
And thank you about friend/member answer. I see what you mean :).
Burgos
+4  A: 

Yes, you should... but you can (and arguably should) implement three of <, >, <=, >= in terms of the other one. This ensures that they behave consistently. Typically < is the one which the others are implemented in terms of because it is the default operator used in sets and maps.

E.g. if you implemented <, you could define >, <= and >= like this.

inline operator>(const Message& lhs, const Message& rhs)
{
    return rhs < lhs;
}

inline operator<=(const Message& lhs, const Message& rhs)
{
    return !(rhs < lhs);
}

inline operator>=(const Message& lhs, const Message& rhs)
{
    return !(lhs < rhs);
}

== and != are often implemented separately. Sometimes classes implement == such that a == b if and only if !(a < b) && !(b < a) but sometimes == is implemented as a stricter relationship that !(a < b) && !(b < a). Doing this does result in more complexity for client of the class, though.

In some situations in can be acceptable to have <, >, <= and >= but not == or !=.

Charles Bailey
Yes, yes, yes! I forgot to mention this but it’s important. You should probably also say that your clean code will be *just as efficient* as coding each comparison manually and independently. This is quite important because it means that any attempt to forego this advice in favour of performance is a premature optimization.
Konrad Rudolph
As such one can also take short-cuts, like `class Message: boost::less_than_comparable<Message, Message>`, and `rel_ops` in `<utility>` (the latter smell somewhat suspicious, since they are made useable by a using declaration). With both `operator<` is enough, and others get implemented in terms of it.
UncleBens
OTOH, trying to use `rel_ops` _can_ be very painful. Typically you have to import them into another namespace and they tend to be far too greedy. If your class is in the global namespace you can end up with `rel_ops` providing comparisons for all sorts of unsuitable types.
Charles Bailey
I think you'd rather use `rel_ops` in the implementation files for *ad hoc* comparison capabilities, rather than to provide comparison operators permanently for a class.
UncleBens
@UncleBens: OK, I misunderstood. I'm still not sure I understand what you mean, though. Do you mean something like `{ using std::rel_ops::operator>; return lhs > rhs; }` as the body of an operator> ?
Charles Bailey
That looks somewhat stupid indeed. Well, perhaps indeed, if your class is in it's own namespace... I've considered it once or twice, but it has made me feel dirty.
UncleBens
+1  A: 

If the assignment doesn't explicitly require the use of operator overloading, you could also consider using a function object. The reason is that there is probably more than one way to compare two Messages for "less-than" (e.g compare contents lexicographically, posting time etc), and therefore the meaning of operator< isn't intuitively clear.

With std::priority_queue the function object to use is specified as the third template parameter (unfortunately you'll also need to specify the second - underlying container type):

#include <queue>
#include <string>
#include <functional>
#include <vector>

class Message
{
    int priority;
    std::string contents;
    //...
public:
    Message(int priority, const std::string msg):
        priority(priority),
        contents(msg)
    {}
    int get_priority() const { return priority; }
    //...
};

struct ComparePriority:
    std::binary_function<Message, Message, bool> //this is just to be nice
{
    bool operator()(const Message& a, const Message& b) const
    {
        return a.get_priority() < b.get_priority();
    }
};

int main()
{
    typedef std::priority_queue<Message, std::vector<Message>, ComparePriority> MessageQueue;
    MessageQueue my_messages;
    my_messages.push(Message(10, "Come at once"));
}

When implementing your own priority queue, you can go about it in the following way:

class MessageQueue
{
    std::vector<Message> messages;
    ComparePriority compare;
    //...
    void push(const Message& msg)
    {
        //...
        if (compare(msg, messages[x])) //msg has lower priority
        //...
    }
};
UncleBens