CODEWITHSUNDEEP
c++
user1527216
user1527216

Reputation: 1173

Overloading comparison operators

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 &'

Upvotes: 2

Views: 3920

Answers (6)

Lol4t0
Lol4t0

Reputation: 12547

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(); 
}

Upvotes: 1

alestanis
alestanis

Reputation: 21863

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; 
}

Upvotes: 7

Nikos C.
Nikos C.

Reputation: 51832

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.)

Upvotes: 1

Luchian Grigore
Luchian Grigore

Reputation: 258548

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).

Upvotes: 3

BigBoss
BigBoss

Reputation: 6914

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();
}

Upvotes: 1

Jason
Jason

Reputation: 1632

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.

Upvotes: 1

Related Questions