Overloaded "==" operator in C++ is not called?

Question

I have a Task class wich has a string text private member. I access the variable trough const string getText() const;.

I want to overload the == operator to check if differents instances of the object have the same text.

I've declared a public bool operator==( const Task text2 ) const; on the class header and code it like this:

bool Task::operator==( const Task text2 ) const {
     return strcmp( text.c_str(), text2.getText().c_str() ) == 0;
}

But it was always returning false even when the strings where equal.

So I added a cout call within the bool operator==( const Task text2 ) const; to check if it was being called, but got nothing.

It seems that my custom == operator is never being called.

My header:

#ifndef TASK_H
#define TASK_H

#include <iostream>

using namespace std;

    class Task {
        public:
            enum Status { COMPLETED, PENDIENT };
            Task(string text);
            ~Task();
            // SETTERS
            void setText(string text);
            void setStatus(Status status);
        // GETTERS
            const string getText() const;
            const bool getStatus() const;
            const int getID() const;
            const int getCount() const;
            // UTILS
            //serialize
            const void printFormatted() const;
            // OVERLOAD
            // = expression comparing text
            bool operator==( const Task &text2 ) const;
        private:
            void setID();
            static int count;
            int id;
            string text;
            Status status;
    };

    #endif

Edited the overload operation to use a reference, and got away from strcmp:

bool Task::operator==( const Task &text2 ) const {
    return this->text == text2.getText();
}

Main file:

using namespace std;

int main() {
    Task *t = new Task("Second task");
    Task *t2 = new Task("Second task");

    cout << "Total: " << t->getCount() << endl;
    t->printFormatted();
    t2->printFormatted();

    if( t == t2 ) {
        cout << "EQUAL" << endl;
    }
    else {
        cout << "DIFF" << endl;
    }

    return 0;
}

Can you post the code where you are *calling* the comparison operator? Also, are you aware that `std::string` implement `operator==`? (no need to call `strcmp` on the C string?) Please add a minimal example that reproduces the problem. — David Rodríguez - dribeas, Oct 06 '12 at 22:23

score 13 · Accepted Answer · answered Oct 06 '12 at 22:29

13

Task *t = new Task("Second task");
Task *t2 = new Task("Second task");
// ...
if( t == t2 ) {

You are not comparing Task objects, but pointers to Task objects. Pointer comparison is native to the language and compares identity of the objects (i.e. will yield true only if the two pointers refer to the same object or both are null).

If you want to compare the objects you need to dereference the pointers:

if( *t == *t2 ) {

answered Oct 06 '12 at 22:29

David Rodríguez - dribeas

204,818
23
294
489

Thanks! as a beginner in C/C++ I'm getting confused sometimes with pointers and references. :) – jviotti Oct 06 '12 at 22:33

score 5 · Answer 2 · answered Oct 07 '12 at 01:43

You wrote:

as a beginner in C/C++ I'm getting confused sometimes with pointers and references.

The solution to that problem is simple: don't use pointers. Unlike C, C++ allows you to write completely useful programs without directly using pointers.

Here is how you could have written your program:

int main() {
    Task t("Second task");
    Task t2("Second task");

    std::cout << "Total: " << t.getCount() << "\n";
    t.printFormatted();
    t2.printFormatted();

    if( t == t2 ) {
        std::cout << "EQUAL\n";
    }
    else {
        std::cout << "DIFF\n";
    }

    return 0;
}

Don't call new. You really didn't need it. As the currently-accepted answer points out, the use of pointers is the root cause of your problem.
Don't use using namespace std;. It introduces subtle bugs (none in your program, but it's best to avoid it.)
Don't use std::endl if you mean '\n'. '\n' means "End this line." std::endl means "End this line and flush the output."

I appreciate a lot this kind of advices. Would you recommend me some book or guide of C++ coding standars or something like that? — jviotti, Oct 07 '12 at 02:13

score 3 · Answer 3 · answered Oct 06 '12 at 22:28

3

You are comparing pointers, not pointed-to objects.

Use if (*t == *t2) or you will simply check if the addresses are the same, which is obviously always false.

answered Oct 06 '12 at 22:28

bitmask

32,434
14
99
159

score 2 · Answer 4 · answered Oct 06 '12 at 22:27

2

You are comparing pointers... Try *t == *t2

answered Oct 06 '12 at 22:27

Xyand

4,470
4
36
63

score 1 · Answer 5 · answered Oct 06 '12 at 22:26

1

No need to define as member function if the getText accessor is public, and there's definitely no need for strcmp, anywhere, ever.

bool operator==(const Task& lhs, const Task& rhs) {
    return lhs.getText() == rhs.getText();
}

answered Oct 06 '12 at 22:26

Puppy

144,682
38
256
465

The book I'm currently reading recommends using operator overloading methods as member functions for terms of speed – jviotti Oct 06 '12 at 22:31
Makes no difference whatsoever. Free functions are far superior in other respects, too. You need to find a new book if it recommends the code you're writing. – Puppy Oct 07 '12 at 09:06

score 1 · Answer 6 · answered Oct 06 '12 at 22:28

1

You're not comparing tasks, you're comparing pointers to tasks. t == t2 does not mean *t == *t2. You cannot overload the == operators for built-in types.

answered Oct 06 '12 at 22:28

score -1 · Answer 7 · answered Oct 06 '12 at 22:15

-1

Try to change the method signature to:

bool Task::operator==(const Task &text2) const;

(i.e., try and use a reference for the text2 parameter).

answered Oct 06 '12 at 22:15

Marco Leogrande

8,050
4
30
48

Was about to mention this. By the way, overloading guidelines brings up that point too: http://courses.cms.caltech.edu/cs11/material/cpp/donnie/cpp-ops.html – Den Oct 06 '12 at 22:18
@JuanCruzViotti With this change, is your operator being called? – Marco Leogrande Oct 06 '12 at 22:24
@JuanCruzViotti You are comparing pointers. Fix your code to compare `*t == *t2` and you will see that it will be called. – Marco Leogrande Oct 06 '12 at 22:29
This is utterly wrong. You are allowed (and you should) actually pass by value and not by reference to allow compiler optimisation. – bitmask Oct 06 '12 at 22:29
1

@bitmask: What optimization would that allow? – Benjamin Lindley Oct 06 '12 at 22:33
If you pass by reference, the compiler has no choice but to pass a pointer internally and dereference that pointer inside the function. If you pass by value the implicit copy can very likely by elided. – bitmask Oct 06 '12 at 22:40
3

@bitmask: If the function takes by value, and it receives an r-value, the copy will be elided, sure. But if it receives an l-value, then there will definitely be a copy. However, if the function takes a reference, there will be no copy in either case. Accepting by value is useful when you need a copy anyway, or when your objects are lightweight. This function doesn't need any copies, and these objects do not seem very lightweight. – Benjamin Lindley Oct 06 '12 at 22:46
At any rate, you *can* (and [sometimes] should) pass by value. – bitmask Oct 06 '12 at 22:52

Overloaded "==" operator in C++ is not called?

7 Answers7