Why can't I use a copy constructor inside of my assignment operator?

Question

Title kind of hits the mark.

    // copy constructor
IntList::IntList(const IntList& source){
    Node* currentNode = source.first;
    first = nullptr;
    while(currentNode){
        append(currentNode->info);
        currentNode = currentNode->next;
    }
}

//Assignment operator should copy the list from the source
//to this list, deleting/replacing any existing nodes
IntList& IntList::operator=(const IntList& source){
    this -> ~IntList();
    this = new IntList(source);
    return *this;
}

Error: intlist.cpp:26:30: error: lvalue required as left operand of assignment this = new IntList(source);

The destructor properly deletes each node in the linked list. Technically "this" would be whatever linked list was passed in with the assignment operator. Isn't that treated as an object that can be set?

Answer 1

The short answer is that you can't assign this. (this is the pointer to the current object it cannot be "relocated", it is a "const" pointer for all purposes).

What you mean is something like:

    new(this) IntList(source);

Which is called "placement" new:

But you don't need to learn about this because all this is bad anyway. Keep reading...

---

Yours is an old and bad design pattern to implement assignment as destruction + construction.

These days, unless there is a better shortcut (that depends on you specific design) we implement swap (also useful) and then operator= from copy(construct)+swap.

(Besides any explict call to the destructor smells bad.)

IntList& IntList::operator=(IntList const& source){
    IntList tmp(source)
    this->swap(tmp);
    return *this;
}

---

The main point is not only more elegant code, ultimately a side effect is that you are writing exception safe code by default. (think of what happens if the new object cannot be constructed in your original code: the *this object is left in an invalid "destructed" state.)

See the series of talks: https://www.youtube.com/watch?v=W7fIy_54y-w

---

This is really advanced now and it gets weird (as discussed with @PaulMcKenzie). It turns out that (I think for some quirks only in C++14), that you can omit the temporary and transfer that into the argument itself passed by value:

IntList& IntList::operator=(IntList source) noexcept{
    return swap(source);
}

Although it can be obscure at first.

1. It is less code
2. it is noexcept: it can't fail!. The operation call can fail but not inside the function
3. this provides move assignment if there a move constructor, so you don't need to write a separate move assignment (even less code).

(Incidentally, I think this shows why member swap should return *this)

This opens an intriguing question: if I have copy constructor and swap, shouldn't the compiler implement operator= automatically for me? (not the old-C operator= member-by-member, but one that knows about the semantics of the class implemented like shown). IntList& operator=(IntList) = default;

---

Finally, don't miss the forest for the tree.

You wanted to implement assignment without writing new code, assuming that your copy constructor was good and that is all you got.

However this is still not always optimal!

For containers copy construction usually involves a) (unconditional) allocation and b) a sequence of "uninitialized copies" (ultimately a form of placement new). Both a) and b) are slower than A) no-allocation and B) a sequence of assignment respectively.

This means that, if for some reason you know that you don't need to allocate (e.g. because the source "fits" in your allocated memory already), and that you can assign element by element (faster than new-place element by element) you might be in good shape to get a more optimal solution by writing a custom operator= that takes into account all this.

Answer 2

If your copy constructor and destructor for IntList are working properly, the easiest way to implement the assignment operator is to use the copy/swap idiom:

#include <algorithm>
//...
IntList& IntList::operator=(const IntList& source)
{
    if (&source != this)
    {
       IntList temp(source);
       std::swap(temp.first, first);
       // add swaps for each member
       //...
    }  // <-- This is where the old contents will be destroyed, 
       // when `temp` goes out of scope.  No need for this->~IntList();
    return *this;
}

In your code, doing this:

this->~IntList();

is not necessary, and probably is wrong anyway. Code like that is reserved for usage with placement-new, and your code doesn't show any usage of placement-new.

Answer 3

A common mistake is to call the assignment operator from the copy constructor. Take the Array class for example. Inside the copy you might think it's simple to write