FAQ 24.04 Should an assignment operator throw an exception after partially assigning an object?

The assignment operator should either completely succeed or leave the object unchanged and throw an exception.

This gives callers some strong assurances: if the assignment operator returns normally (as opposed to throwing an exception), the caller is assured that the assignment was completely successful; if the assignment operator throws an exception, the caller is assured that the object was left in a consistent state.

Sometimes achieving this goal means that the object's new state must be copied into some temporary variables; then, after all potential error conditions are bypassed, the state can be copied into the this object.

In any event, it would be a mortal sin to leave the object in a corrupt state when an exception is thrown. For example, if an exception is thrown while evaluating new Wilma(*f.p_) (that is, either an out-of-memory exception or an exception in Wilma's copy constructor), this->p_ will be a dangling pointer it will point to memory that has already been deleted.

 #include <new> using namespace std; class Wilma { }; class Fred { public:   Fred()                          throw(bad_alloc);   Fred(const Fred& f)             throw(bad_alloc);  ~Fred()                          throw();   Fred& operator= (const Fred& f) throw(bad_alloc);   //... private:   Wilma* p_; }; Fred::Fred()              throw(bad_alloc) : p_(new Wilma()) { } Fred::Fred(const Fred& f) throw(bad_alloc) : p_(new Wilma(*f.p_)) { } Fred::~Fred()             throw()          { delete p_; } Fred& Fred::operator= (const Fred& f) throw(bad_alloc) {   if (this == &f)                                    <-- 1     return *this;   delete p_;                                         <-- 2   p_ = new Wilma(*f.p_);   return *this; } 

(1) Check for self-assignment: GOOD FORM!

(2) Delete the old before allocating the new: BAD FORM!

The easiest way to solve this problem is simply to reverse the order of the new and delete lines. That is, to allocate the new Wilma(*f.p_) first, storing the result into a temporary Wilma*, then to do the delete p_ line, and finally to copy the temporary pointer into p_:

 Fred& Fred::operator= (const Fred& f) throw(bad_alloc) {   if (this == &f)                                    <-- 1     return *this;   Wilma* p2 = new Wilma(*f.p_);   delete p_;                                         <-- 2   p_ = p2;   return *this; } 

(1) Check for self-assignment: GOOD FORM!

(2) Allocate the new before deleting the old: GOOD FORM!

Note that reversing the order of the new and delete statements has the beneficial side effect of making self-assignment harmless even without the explicit if test at the beginning of the assignment operator. That is, if the initial if test is removed and someone does a self-assignment, the only thing that happens is an extra copy of a Wilma object. Since making an extra copy of a Wilma object shouldn't generally cause problems (especially since the caller was expecting a copy to be made during assignment anyway), the if test can be removed, thus simplifying the code and improving the performance in the normal (non-self-assignment) case. Remember: the goal is to make self-assignment harmless, not to make it fast. Never optimize the pathological case (self-assignment) at the expense of the normal case (non-self-assignment). (See FAQ 24.03.)

The modified assignment operator follows.

 Fred& Fred::operator= (const Fred& f) throw(bad_alloc) {   // Self assignment is handled by order of first three lines:   Wilma* p2 = new Wilma(*f.p_);   delete p_;   p_ = p2;   return *this; }