c++ copy assignment syntax comparison - which is better?

Question

I am studying C++ at University, and in the break I am going through Strousrtup's "The CPP Programming Language 4th Edition" to fill in the gaps of my understanding and what we are being taught in class.

In section 3.3.1 he details a code snippet for a simplified version of a vector class (which is type-specific to doubles only):

Vector& Vector::operator=(const Vector& a) {
    double* p = new double[a.sz]; 
    for (int i=0; i!=a.sz; ++i)
        p[i] = a.elem[i]; 
    delete[] elem;
    elem = p;
    sz = a.sz;
    return *this;
}

Now, I had already written my own version of an overridden copy assignment operator to go along with this simplified quasi-vector before I saw this, which seems to work correctly, but I was wondering, is there anything wrong with deleting the memory allocated that elem points to and then re-initialising it, as I do below, compared to how Stroustrup does it?

vectoR& vectoR::operator=(const vectoR& v) {
    delete[] elem;
    elem = new double[v.size()];
    sz = v.size();
    for (int i = 0; i != sz; ++i) 
        elem[i] = v[i];
    return *this;
}

Answer 1

Yes, Strousrtup's way is self-assignment safe. That is, an instance can be assigned to itself

a = a;

Once you've finished that book you may want to scoot through Meyer's "Effective C++" (2005) which is also an excellent text and considers such problems as these.

Answer 2

Stroustrup's implementation will not destroy the existing elem if an exception occurs, and allows self-assignment.

Answer 3

Your version is incorrect, and has undefined behavior. What happens if new double[v.size()] throws an exception?

In general, you shouldn't do anything which can invalidate an object until after you've done everything which might throw and exception. Leaving a point to deleted memory results in an invalid object, that new can always throw, so you shouldn't delete elem until after you've done the new.

EDIT:

To be more explicit: from the original poster's suggested implementation:

delete[] elem;
elem = new double[v.size()];

The first line invalidates the pointer elem, and if there is an exception in the second line (and new can always throw an exception), then the assignment operator leaves the object with the invalid pointer; any further access to this pointer, including in the destructor of the object, is undefined behavior.

There are, in fact, many ways of avoiding this problem in this particular instance:

delete[] elem;
elem = nullptr;
elem = new double[v.size()];

for example (provided that any functions called on the object can deal with a null pointer), or (what is effectively the same thing):

delete[] elem;
elem = new (std::nothrow) double[v.size()];
if ( elem == nullptr )
    throw std::bad_alloc();

Both of these solutions are in many ways special, however, and not generally applicable. They also leave the object in a special state, which may require extra handling. The usual solution is to do anything that can throw before modifying any of the state of the object. In this case, the only thing which can throw is the new, and we end up with Stroustrup's solution. In more complicated objects, the necessary solution may be more complicated; one common simple solution is the swap idiom:

MyType& MyType::operator=( MyType const& other )
{
    MyType tmp( other );    //  Copy constructor
    swap( tmp );            //  Member function, guaranteed nothrow
    return *this;
}

This works well if you can write a nonthrow swap member function. You often can, because swapping pointers is a nothrow (so in this case, all swap would do is swap elem), but it is not a given. Each case needs to be evaluated individually.

The swap idiom does give the "strong" guarantee: either the assignment fully succeeds, or the object's state is unchanged. You don't often need this guarantee, however; it's usually sufficient that the object be in some coherent state (so that it can be destructed).

Finally: if your class has several resources, you'll almost certainly want to encapsulate them in some sort of RAII class (e.g. smart pointer) or in separate base classes, so that you can make the constructors exception safe, so that they won't leak the first resource if allocating the second fails. This can be a useful technique even in cases where there is only one resource; in the original example, if elem had been an std::unique_ptr<double[]>, no delete would have been necessary in the assignment operator, and just:

elem = new double[v.size()];
//  copy...

is all that would be needed. In practice, if real code, cases where this solves the solution are fairly rare; in real code, for example, the orginal problem would be solved with std::vector<double> (and the requirements of std::vector are such that std::unique_ptr is not really a solution). But they do exist, and classes like std::unique_ptr (or an even simpler scoped pointer) are certainly a solution worth having in your toolkit.