How to correctly write an assignment operator for a derived class

Question

Answers to other questions say this is how to write assignment for a derived class:

class D : public B
{
public:
    D& operator=(const D& other)
    {
        B::operator=(other);
        // D-specific stuff
        return *this;
    }
};

But shouldn't the usual "check for self assignment" be done? Eg:

class D : public B
{
public:
    D& operator=(const D& other)
    {
        if ( this != &other ) {
            B::operator=(other);
            // D-specific stuff
        }
        return *this;
    }
};

Or maybe the base class assignment should be left out of the check?

"But shouldn't the usual "check for self assignment be done?" - should — kotlomoy, May 23 '13 at 19:28
I would follow the [copy-swap-idiom](http://stackoverflow.com/q/3279543/942596) for assignment. — andre, May 23 '13 at 19:33
@JohnH.: The same as in any other case. Your swap function simply calls swap for all members (including the base class) — Grizzly, May 23 '13 at 19:45

score 4 · Answer 1 · answered May 23 '13 at 19:37

4

Warning - the following may sound a bit hard-line.

But shouldn't the usual "check for self assignment" be done?

Emphatically, no.

Consider the circumstances under which self-assignment would occur. Something as contrived as:

D d;
d = d;

But the above code is flawed, and should never have been written in the first place. It's the same idea as having a global catch (...) intended to catch all unhandled exceptions. If that global catch-all is going to try to continue on as if nothing had happened, all that you are going to get is a nightmare. You won't prevent the program from crashing or doing something even worse (like killing the patient or selling when you should have bought 10k shares of IBM), you just prevented it from crashing right now.

Self-assignment, as in the code sample above, is similarly flawed. Checking for self-assignment and continuing on as if nothing had happened is just sticking your head in he sand, pretending everything is just peachy. The reality is, there is a bug in your code that needs to be fixed.

If self-assignment should never occur, it should (could) be an assertion. It should not just roll along. Find and fix the bug that causes self-assignment. Don't code around it.

answered May 23 '13 at 19:37

John Dibling

99,718
31
186
324

1

More importantly than blocking self assignment is making sure your code would work even if self assigned: there are few bugs that are self-assignment only, but many relatively obscure bugs show up in self-assignment reliably. – Yakk - Adam Nevraumont May 23 '13 at 19:39
Note that modern typical implementations of the assignment operator (ie. either the default one or copy-and-swap) have no problem other than efficiency on self-assignments. – Alexandre C. May 23 '13 at 19:40
So you are saying that `d = d` *shouldn't* work? That would really go against the principle of least suprise (It's not what `int` does). Besides it isn't necessarily something as contrieved as your example, particulary if you consider that the arguments might come from out of different pointers. Now you could code around that or specify for each algorithm that it doesn't work in cases of pointer aliasing, but that only serves to make everything more complex (there is nothing inheritly wrong with code involving self assignment). – Grizzly May 23 '13 at 19:57
@Grizzly: That's not exactly what I'm saying. What I'm saying is, if self-assignment would cause a problem **and** you expect it to never happen, then you should not check for it at run-time. You should possibly assert for it, possibly unit test it, and let the program die if it does happen at run-time. – John Dibling May 23 '13 at 20:00
I'm not following, exactly. At first I thought you were saying self-assignment should never occur, so operator= should always check for and reject it. But then your last paragraph starts "_If_ self-assignment should never occur", implying that it is ok sometimes. – John H. May 23 '13 at 20:00
@JohnH.: There is nothing *universally* wrong with self-assignment. However the applications of self-assignment are fairly scarce. Much more often, self-assignment occurs as a result of a programming defect, often obscure. So, self-assignment is OK so long as it is what you intended. If it's not what you intended, then it should be impossible and you do not test at run-time for things that are impossible. That's what unit tests and assertions are for. – John Dibling May 23 '13 at 20:06
@JohnH.: (Continuing) In your OP code, you were checking for self-assignment and NOPing if it happened. If you NOPed because self-assighnent should never happen, then you shouldn't have that run-time check. If you NOP for reasons of efficiency, then that's another matter entirely. Unusual, but another matter. – John Dibling May 23 '13 at 20:09
I still don't understand. I have a class. I can write assignment to properly accommodate self-assignment. I have no idea how somebody might want to use my class. So shouldn't I allow self-assignment? – John H. May 23 '13 at 20:14
@JohnDibling: Why would you expect it to never happen? In my experience self assignment is typically a consequence of not doing some special handling for such case, thus simplifying the code using the class (which is a good thing). On the other side I can't remember the last time I saw an actual defect due to self assignment happening. Now considering that you typically don't necessarily control the code using your class, why would you assume that it should never occur? – Grizzly May 23 '13 at 20:16
@JohnH.: Just use the copy-and-swap idiom. That way you don't have to worry about self assignment, since it just works. Otherwise just check for self assignment and handle it normally. – Grizzly May 23 '13 at 20:18

How to correctly write an assignment operator for a derived class

1 Answers1