Overloading comparison operators

Question

I'm having trouble overloading the comparison operators > and <. I've tried two different ways but I'm still having trouble.

bool Car::operator ==(const Car &car)
{
    return mLNumber == car.GetNum();
}

bool Car::operator <(const Car &carB)
{
    return mLNumber < carB.GetNum(); 
}
bool Car::operator >(const Car &carB)
{
    return mLNumber > carB.GetNum(); 
}
int Car::GetNum()
{
    return mLNumber;
}

My == operator works just fine. I get the error that these operators don't exist. Here is my 2nd attempt.

bool Car::operator <(const Car &carA, const Car &carB)
{
    return carA.GetNum() < carB.GetNum(); 
}
bool Car::operator >(const Car &carB)
{
    return carA.GetNum() > carB.GetNum(); 
}

And I get the error that there are too many parameters. I also get this:

'Car::GetNum' : cannot convert 'this' pointer from 'const Car' to 'Car &'

alestanis · Accepted Answer · 2012-10-25 20:38:43Z

7

Try making your operators const:

bool Car::operator <(const Car &carB) const {
    return mLNumber < carB.GetNum(); 
}
bool Car::operator >(const Car &carB) const {
    return mLNumber > carB.GetNum(); 
}

Edit: And in this case, you should make the GetNum() function as const too because you are calling it on const Car& objects.

You don't need GetNum() either, you could just write

bool Car::operator <(const Car &carB) const {
    return mLNumber < carB.mLNumber; 
}

edited Oct 25, 2012 at 20:38

answered Oct 25, 2012 at 20:18

alestanis

22k4 gold badges50 silver badges67 bronze badges

Sign up to request clarification or add additional context in comments.

8 Comments

Nikos C. Over a year ago

No, that's not the problem here. The problem is that GetNum() isn't const. The operators themselves aren't actually getting called on any const object.

Luchian Grigore Over a year ago

@NikosChantziaras you should revise your downvoting policy - this is merely incomplete, not wrong. As is yours (the operators should be const, + you're not treating the other question - the error on multiple parameters). +1 from me because you were first (but still incomplete)

Nikos C. Over a year ago

This doesn't solve the OPs problem at all, which is why I thought this is a bad answer. It's actually surprising that everyone else also didn't detect the real reason for the error. Of course the operators should be const, but it's not the actual problem here :-)

Luchian Grigore Over a year ago

@NikosChantziaras sometimes, the slightest suggestion can be useful. Did it occur to you that making the operators const will give a compiler error cannot call non-const method GetNum? And make it clear that GetNum must be made const. Also, the second part actually does answer the question, since it removes GetNum altogether.

Luchian Grigore Over a year ago

@NikosChantziaras life's unfair like that. </sarcasm>

|

Luchian Grigore · Accepted Answer · 2012-10-25 20:24:37Z

3

There's two problems in the code - first, logically, your operators are immutable on the type - they don't change the objects, they only analyze them, and you should be able to call them on immutable (const) objects. So, as alestanis pointed out, make them const (all operators and the getter method).

Second, < == and > are binary operators. There's two options to implement them: as free operators, or as members. You went with members, which is okay, but:

bool Car::operator <(const Car &carA, const Car &carB)

doesn't declare a binary operator. When you implement an operator as a member, the first parameter is implicitly the current object (*this), so it should be

bool Car::operator <(const Car &carB) const

The free operator would look like:

bool operator < (const Car& carA, const Car& carB);

Note that const on this one doesn't make sense since it's not a member. However, note that the first parameter (carA) is marked as const, which corresponds to the const applied to the method in the member version (which, under the hood, marks this as const).

answered Oct 25, 2012 at 20:24

Luchian Grigore

260k68 gold badges465 silver badges634 bronze badges

3 Comments

Nikos C. Over a year ago

That's not the problem here, since the operators in this particular case aren't called on const objects. The real problem is that GetNum() isn't const, so it can't be called on the carB object, which is const.

Luchian Grigore Over a year ago

@NikosChantziaras did you read the whole answer? What about the "make them const (all operators and the getter method)" part? Being short-sighted and treating only the immediate errors are the trait of a beginner. So excuse me for going the extra mile!

JAYD3V Over a year ago

This is a great answer, IMO, I was happy to here an extra mile long answer. How is this answer only +2 and so old?

Jason · Accepted Answer · 2012-10-25 20:20:27Z

1

It would help to see where you're calling these operators.

Your second version with two Car arguments should be global operators. So drop the Car:: from the definitions if you use them. Move the declarations out of the Car class' body, too.

Since your calling GetNum on a const Car object, the Car::GetNum function also needs to be const. The dirtier way to do this is to cast away the const-ness, but that is frowned upon.

answered Oct 25, 2012 at 20:20

Jason

1,63217 silver badges24 bronze badges

Comments

BigBoss · Accepted Answer · 2012-10-25 20:21:15Z

1

Your error is when you want to compare one const object and you operators is not marked asconst, as a general rule, you should always mark functions and operators that do not change your object as const and this make life much easier. For example:

bool Car::operator ==(const Car &car) const // <-- This function is const
{
    return mLNumber == car.GetNum();
}

answered Oct 25, 2012 at 20:21

BigBoss

6,9243 gold badges26 silver badges39 bronze badges

Comments

Nikos C. · Accepted Answer · 2012-10-25 20:27:27Z

The problem is that car::GetNum() isn't declared const, so you can't call it on const instances of car. The operators take a const Car &carB as argument, so you can't call GetNum() on carB, since carB is a const object, but GetNum() has not been declared const.

You should get into the habit of declaring all functions that don't modify the object as const. To declare a function as const, simply append const after the closing parenthesis. Both in the declaration, as well as the definition. For example:

class car {
    // ...
    void car::foo() const;
    // ...
};

void car::foo() const
{ /* ... */ }

Or, if you're defining it inline inside the class declaration:

class car {
    // ...
    void car::foo() const
    { /* ... */ }
    // ...
};

Although not strictly necessary in this particular case (meaning this isn't why the code doesn't compile), the operators themselves should also be declared const for the same reason (so that you can use them also on const objects.)

Lol4t0 · Accepted Answer · 2012-10-25 20:38:52Z

1

In the first case:

Your operators do not change data of operands, then it should be made const:

bool Car::operator <(const Car &carB) const {
                                    //^^^^^
    return mLNumber < carB.GetNum(); 
}
bool Car::operator >(const Car &carB) const {
                                   // ^^^^^
    return mLNumber > carB.GetNum(); 
}

int Car::GetNum() const
                //^^^^^
{
    return mLNumber;
}

In the second case, when operators accept 2 arguments, they should be implemented as free functions:

bool operator <(const Car &carA, const Car &carB)
{
    return carA.GetNum() < carB.GetNum(); 
}
bool operator >(const Car &carA, const Car &carB)
{
    return carA.GetNum() > carB.GetNum(); 
}

edited Oct 25, 2012 at 20:38

answered Oct 25, 2012 at 20:21

Lol4t0

12.6k4 gold badges32 silver badges67 bronze badges

Collectives™ on Stack Overflow

Overloading comparison operators

6 Answers 6

8 Comments

3 Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

8 Comments

3 Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related