Is it really safe not to check in the copy assignemt operator whether an object is assigned to itself?

Question

Here is an example implementing "Rule of Three" which I found:

class Array { 
    public: 
        int size; 
        int* vals;
        Array() : size(0), vals(NULL){}
        Array( int s, int* v );
        Array(const Array&); // 1
        Array& operator=(const Array&); // 2
        ~Array(); // 3
}; 

Array::~Array() { 
   delete []vals; 
   vals = NULL; 
} 

Array::Array( int s, int* v ){
    size = s; 
    vals = new int[size]; 
    std::copy( v, v + size, vals ); 
} 

Array::Array(const Array& rhs):
    size(rhs.size),
        vals((rhs.size) ? new int[size] : NULL)
{
    if(size)
        std::copy(rhs.vals, rhs.vals + rhs.size, vals);
}

Array& Array::operator=(const Array& rhs){
//  if(this == &rhs) // no need
//      return *this;
    int* a = (rhs.size)? new int[rhs.size] : NULL; // this is why we don't need the above check: if this line throws then vals is untouched. if this is ok (local variable) then continue (next won't throw).
    std::copy(rhs.vals, rhs.vals + rhs.size, a); // copying to a even (self assignment won't harm)
    delete[] vals;
    vals = a;
    size = rhs.size;

    return *this;
}

As you can see above the check in the Copy Assignment Operator is removed as the fact being creating a local variable then delete the member pointer and assign it the local one. But what matters me if I write:

int main() {
   int vals[ 4 ] = { 11, 22, 33, 44 };  
   Array a1( 4, vals ); 

   a1 = a1; // here I assigned a1 to itself

    return 0;
}

I assigned a1 to itself does this mean a1's valsis removed and then assigned the local one a in the Copy assignment operator? Is this a correct way? Does my code has some pitfalls?

Any tip, advice is highly appreciated.

What problems does `a1 = a1;` give you? Your copy assignment operator looks to handle it properly, but not efficiently. — NathanOliver, Dec 10 '18 at 22:05
*Most* (there are possible "exceptions") code that *needs* (for correctness) that check is not exception safe and might need a catch clause to not leave the object in a bad state in case new throws. — curiousguy, Dec 10 '18 at 22:06
@Maestro Checking for self assignment. If you are assigning to yourself, just do nothing. That's O(1) instead of the O(N) non checking way. — NathanOliver, Dec 10 '18 at 22:08
Do note that what you have done is basically manually implement the [copy and swap idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). If you are okay with the copy cost on self assignment, you might want to use it as, IMHO, it is cleaner. — NathanOliver, Dec 10 '18 at 22:13

score 8 · Accepted Answer · answered Dec 10 '18 at 22:09

The copy assignment operator will function, in the sense that self-assignment will have the expected behavior: the value content of the array will be unchanged.

What you're losing by not doing the test is that, if you already hold an array, you will allocate an array you don't need, perform a copy you don't have to do, then destroy something you could have kept.

Whether this matters for performance... that will depend heavily on usage-dependent factors. After all, this is only a problem if you perform a self-copy. How often do you do that? And if you really don't do it that often (or ever), then the test is just a conditional branch you don't need.

At the same time, consider the definition of vector. It has very specific circumstances for when iterators/pointers/references into the vector become invalidated. And copying the vector is not one of them. So if the location of object storage is a part of the interface of your array type (the way it is for vector), then doing self-copies has broader implications for your interface.

Thank you get it. One last question: So whenever I achieve the "Rule of Three" do I need to use "Copy and Swap"? — Maestro, Dec 11 '18 at 19:38

Is it really safe not to check in the copy assignemt operator whether an object is assigned to itself?

1 Answers1