1
class Person {
private:
    string name;
    int id;

public:
    Person(string name, int id): name(name), id(id) {}
    const Person& operator=(const Person &another) {
        if (*this == another) // What is the problem here?
            return *this;
        // do copy
        return *this;
    }
};

I want to make a operator= overloading function. In the self assignment checking, if I do the checking as above, it will show error saying Invalid operands to binary expression (Person and const Person). But if I do this == &another, no error will be shown.
Is the error saying that type of this and type of another are different? But if so, how come this == &another will work?

keanehui
  • 185
  • 2
  • 13
  • 3
    You're trying to call `operator==`, which is not generated for you, so you must have written but have not shown. There is a built-in equality operator for pointers. – Useless Jun 01 '20 at 10:49
  • In my experience, it is better to make the assignment operator be self-assign safe rather than to optimize for the pathological case. – Eljay Jun 01 '20 at 12:32

2 Answers2

2

Self assignment checking is done by comparing addresses of the two objects

if (this == &another)

C++ has a built in operator== for pointers of the same type.

Your code is comparing the values of the two objects, and clearly you haven't defined operator== for your Person class.

But comparing the values is not the right thing to do because there's no special action needed when you are assigning two objects which have the same value, but sometimes there is when you are assigning two identical objects.

In fact there's no need in the class you have shown to do any self-assignment testing. Simply this is OK

Person& operator=(const Person &another) {
    name = another.name;
    id = another.id;
    return *this;
}

Don't think that testing for self-assignment is any kind of efficiency gain, because self-assignment is rare. So if you can avoid the test, then generally you should.

Also worth saying that the generally prefered method for assignment is the copy and swap idiom which also doesn't require any self-assignment testing.

john
  • 85,011
  • 4
  • 57
  • 81
1

In *this == another you are trying to check whether both objects have the same value. For this to work, you have to define the operator== for Person. That is, Person::operator==() will determine whether two Person objects have the same value.

However, since you want to protect against self-assignment, what you really need is to compare the identity of both objects – as opposed to their value. You can achieve this by comparing their addresses in memory, i.e.:

if (this == &another) // Is "another" the same object?
   return *this; // skip assignment

The operands to this operator== are pointers to Person, not Person objects.

JFMR
  • 23,265
  • 4
  • 52
  • 76