Can't find mistake in overloading operator<

Question

I tried to overload operator < for sorting the rooms based on price.

Here is the relevant part of my code:

class Room{
    protected:
        int roomNo;
        int category;
        Client client;
    public:
        Room();
        Room(int no, int cat, Client cl);
        void printData();
        int charge();
        Room operator < (Room &r1);
};

Room Room::operator < (Room &r1){
    if(this->charge() < r1.charge()){
        return r1;
    }
    else{
        return *this;
    }
}

But the compiler gives me the following error when I try to use operator<:

main.cpp:(.text+0x2d8): undefined reference to `Room::operator<(Room const&)'
main.cpp:(.text+0x2ff): undefined reference to `Room::operator<(Room const&)'
[Error] ld returned 1 exit status
Makefile.win    recipe for target 'Ask01.exe' failed

Why doesn't the code compile?

Please look at http://stackoverflow.com/questions/4421706/operator-overloading?rq=1, your function signature is incorrect. Hint: the error tells you what it should be — EdChum, May 12 '16 at 12:43
Shouldn't that be `Room& Room::operator < (const Room& r1)`? — trojanfoe, May 12 '16 at 12:44
Whet I tried that i got this: " 39 7 [Error] prototype for 'Room& Room::operator<(Room&)' does not match any in class 'Room'" — Sini, May 12 '16 at 12:46
That's because the declaration is also wrong. Note parameter is `const`. — trojanfoe, May 12 '16 at 12:46
If I use const i cant use the function charge because it's a non-static function — Sini, May 12 '16 at 12:49
You mean it's not `const`. It doesn't modify `Room` so it should be. — trojanfoe, May 12 '16 at 12:50
Please refrain from using the `this->` syntax. C++ is not Java, C# or Smalltalk. If you need to access a member variable, access it directly. If your parameter names are the same as the member names, change them. — Thomas Matthews, May 12 '16 at 16:44

score 4 · Answer 1 · edited May 23 '17 at 11:44

4

Your class Room has a member functon with signature Room operator<(Room& r1). However, this is not signature to be used as overloaded operator<. Change your definition to

Room operator<(const Room& r1);

And your declaration to

Room Room::operator<(const Room& r1)
{
    /* code */
}

Also, it is considered better practice to overload operators as friend non-member functions (See this question).

Also, I would consider to change return value of operator< to const Room& to avoid certainly unnecessary copy.

Anyways, why do the return value of operator< is not bool? It might confuse someone.

Room first, second;
if (first < second)
    doSomething(first);
else
    doSomething(second);

might be little longer than simple (first < second).doSomething(), but your intuition is definitely clearer.

Also, if you insist on defining operator< as member function, I would recommend to declare it as const (like bool /*Room*/ operator<(const Room& r1) const;. You would have to declare as const also int charge() and maybe even Client::getTheD). Look at more about const correctness.

edited May 23 '17 at 11:44

Community

1
1

answered May 12 '16 at 12:52

Zereges

5,139
1
25
49

3

Shouldn't we recommend that the return type of `operator <` be `bool`? – John Zwinck May 12 '16 at 12:53
@JohnZwinck good point :) – Zereges May 12 '16 at 12:54
Actually that is what i did (the last one) but i had a mistake in a function in main() and the compiler didnt catch it so it resolved in a makefile error – Sini May 12 '16 at 13:02
Minor correction: `operator < ()` should be `const`. – mindriot May 12 '16 at 13:03
Also I use Room instead of bool so i can make inline statements of the , eg (r1 < (r2 < r4)) – Sini May 12 '16 at 13:03
@Sini By the way, with regards to your original plan (return the smaller of the two elements): Once you have defined proper comparison operators, you can use [`std::max()`](http://en.cppreference.com/w/cpp/algorithm/max) and [`std::min()`](http://en.cppreference.com/w/cpp/algorithm/min) to get what you want. – mindriot May 12 '16 at 13:05
@mindriot It should in general, but is is not a condition. In OP's case, he would have to declare `int charge()` as `const` as well and maybe even `Client::getTheD`. – Zereges May 12 '16 at 13:06
@Zereges you're right that it is not a condition (hence I said _should_, not _must_). But [const correctness](https://isocpp.org/wiki/faq/const-correctness) is strongly recommended (hence my comment). – mindriot May 12 '16 at 13:08
@mindriot I agree with you, but the code would not compile as is (hence my comment). – Zereges May 12 '16 at 13:12
@Zereges true, fair enough :) – mindriot May 12 '16 at 13:13
Thank you all , the code did actually run , my mistake was an invalid call to a function that the compiler didnt catch . But yes I should have made it const :) – Sini May 12 '16 at 13:39

Can't find mistake in overloading operator<

1 Answers1