CODEWITHSUNDEEP
c++move-assignment-operator
tannic
tannic

Reputation: 39

Move constructor vs. Move assignment

As an extension to This question, i am trying to get my move assignment correct.

I have the following code:

// copy assignment operator
LinkedList<T>& operator= (LinkedList<T> other) noexcept
{
    swap(*this, other);
    return *this;
}

// move assignment operator
LinkedList<T>& operator= (LinkedList<T>&& other) noexcept
{
    swap(*this, other);
    return *this;
}

But when i try to use it, my code fails to compile.

First some code:

LinkedList<int> generateLinkedList()
{
    LinkedList<int> List;   
    List.add(123);
    return List;
}


int main()
{
    LinkedList<int> L;   
    L = generateLinkedList();
      ^ get an error here...

I get the following error:

main.cpp(24): error C2593: 'operator =' is ambiguous

linkedlist.h(79): note: could be 'LinkedList &LinkedList::operator =(LinkedList &&) noexcept'(Points to the move assignment operator)

linkedlist.h(63): note: or 'LinkedList &LinkedList::operator =(LinkedList) noexcept' (Points to the copy assignment operator)

main.cpp(24): note: while trying to match the argument list '(LinkedList, LinkedList)'

Are my move assignment operator wrong, or do i use it the wrong way?

Upvotes: 1

Views: 246

Answers (1)

Max Langhof
Max Langhof

Reputation: 23681

The copy assignment operator would take a const LinkedList<T>& other, not a LinkedList<T> other.

This

LinkedList<T>& operator= (LinkedList<T> other) noexcept
{
    swap(*this, other);
    return *this;
}

is how one would implement both copy and move assignment at once using copy-and-swap. By re-using the copy and move constructors (other is either copy-constructed or move-constructed), and you just swap this with other. other dies at the end of the function, taking with it the old state of this. This implementation is totally fine, but then you don't need a second overload for temporaries (which is indeed ambiguous).

If you want to provide separate copy assignment operators for copy vs move assignment, the signatures would be

// copy assignment operator
LinkedList<T>& operator=(const LinkedList<T>& other) noexcept
{
  //...
}

// move assignment operator
LinkedList<T>& operator=(LinkedList<T>&& other) noexcept
{
  //...
}

But since you already have swap and the copy+move constructors, using copy-and-swap is preferable.

PS: As these appear to be inline definitions (i.e. within the class body), you can skip the <T> template argument - within the LinkedList template class definition, writing LinkedList automatically refers to the "current instantiation" (i.e. LinkedList<T>).

Upvotes: 5

Related Questions