jviotti
jviotti

Reputation: 18919

Overloaded "==" operator is not called when comparing pointers

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

I want to overload the == operator to check if different 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;
}

Upvotes: 4

Views: 5447

Answers (7)

Robᵩ
Robᵩ

Reputation: 168626

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

  2. Don't use using namespace std;. It introduces subtle bugs (none in your program, but it's best to avoid it.)

  3. Don't use std::endl if you mean '\n'. '\n' means "End this line." std::endl means "End this line and flush the output."

Upvotes: 5

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

Upvotes: 13

bitmask
bitmask

Reputation: 34628

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.

Upvotes: 3

user743382
user743382

Reputation:

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.

Upvotes: 1

Xyand
Xyand

Reputation: 4488

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

Upvotes: 2

Puppy
Puppy

Reputation: 146930

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

Upvotes: 1

Marco Leogrande
Marco Leogrande

Reputation: 8458

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

Upvotes: -1

Related Questions