What is wrong with "checking for self-assignment" and what does it mean?

Question

In Herb Sutter's book Exceptional C++ (1999), he has words in item 10's solution:

"Exception-unsafe" and "poor design" go hand in hand. If a piece of code isn't exception-safe, that's generally okay and can simply be fixed. But if a piece of code cannot be made exception-safe because of its underlying design, that almost always is a signal of its poor design.

Example 1: A function with two different responsibilities is difficult to make exception-safe.

Example 2: A copy assignment operator that is written in such a way that it must check for self-assignment is probably not strongly exception-safe either

What does he mean by the term "check for self-assignment"?

[INQUIRY]

Dave and AndreyT shows us exactly what "check for self-assignment" means. That's good. But the question is not over. Why does "check for self-assignment" hurts "exception safety"(according to Hurb Sutter)? If the caller tries to do self-assignment, that "check" works as if no assignment ever occurs. Does it really hurt?

[MEMO 1] In item 38 Object Identity later in Herb's book, he explains about self-assignment.

Answer 1

A question that's of greater importance in this case would be:

---

• "What does it mean, when a you write function in a way, that requires you to check for self assignment???"

---

To answer my rhetorical question: It means that a well-designed assignment operator should not need to check for self-assignment. Assigning an object to itself should work correctly (i.e. have the end-effect of "doing nothing") without performing an explicit check for self-assignment.

For example, if I wanted to implement a simplistic array class along the lines of

class array {
    // code...
 
    int *data;
    size_t n;
};

...and came up with the following implementation of the assignment operator...

array &array::operator =(const array &rhs) 
{
    delete[] data;

    n = rhs.n;
    data = new int[n];

    std::copy_n(rhs.data, n, data);

    return *this;
}

That implementation would be considered "bad" since it obviously fails in case of self-assignment.

---

In order to "fix" this, you have two options;

1. Add an explicit self-assignment check

array &array::operator =(const array &rhs) {
    if (&rhs != this) {
        delete[] data;

        n = rhs.n;
        data = new int[n];

        std::copy_n(rhs.data, n, data);
    }
    return *this;
}

2. Follow a "check-less" approach:

array &array::operator =(const array &rhs) {
      size_t new_n = rhs.n;
      int *new_data = new int[new_n];

      std::copy_n(rhs.data, new_n, new_data);

      delete[] data;

      n = new_n;
      data = new_data;

      return *this;
}

---

The latter approach is better in a sense that it works correctly in self-assignment situations without making an explicit check. This implementation is far for perfect from a 'safety point of view', it is here to illustrate the difference between "checked" and "check-less" approaches to handling self-assignment. The later check-less implementation can be written more elegantly through the well-known copy-and-swap idiom.

This does not mean that you should avoid explicit checks for self-assignment. Such check do make sense from the performance point of view: there's no point in carrying out a long sequence of operations just to end up "doing nothing" in the end. But in a well-designed assignment operator such checks should not be necessary from the correctness point of view.

Answer 2

From c++ core guide

Foo& Foo::operator=(const Foo& a)   // OK, but there is a cost
{
    if (this == &a) return *this;
    s = a.s;
    i = a.i;
    return *this;
}

This is obviously safe and apparently efficient. However , what if we do one self-assignment per million assignments? That's about a million redundant tests (but since the answer is essentially always the same, the computer's branch predictor will guess right essentially every time). Consider:

Foo& Foo::operator=(const Foo& a)   // simpler, and probably much better
{
    s = a.s;
    i = a.i;
    return *this;
}

Note: The code above only apply to classes without pointer, for classes with pointer point to dynamic memory. Please refer to Ant's answer.

Answer 3

MyClass& MyClass::operator=(const MyClass& other)  // copy assignment operator
{
    if(this != &other) // <-- self assignment check
    {
        // copy some stuff
    }

    return *this;
}

Assigning an object to itself is a mistake but it shouldn't logically result in your class instance changing. If you manage to design a class where assigning to itself changes it, it's poorly designed.

Answer 4

The general reason to check for self-assignment is because you destroy your own data before copying in the new one. This assignment operator structure is also not strongly exception safe.

As an addendum, it was determined that self-assignment does not benefit performance at all, because the comparison must run every time but self-assignment is extremely rare, and if it does occur, this is a logical error in your program (really). This means that over the course of the program, it's just a waste of cycles.