1

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?

Community
  • 1
  • 1
tannic
  • 37
  • 5
  • Good explanation in the C++11 section of the first answer to [What is the copy-and-swap idiom?](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). Might even be good enough. – user4581301 Nov 26 '19 at 15:59

1 Answers1

5

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>).

Max Langhof
  • 23,383
  • 5
  • 39
  • 72
  • Now i have updated the copy assignment operator to receive a const LinkedList&. Now that wont compile. I was following the examples in (https://stackoverflow.com/questions/3279543) – tannic Nov 26 '19 at 16:55
  • @tannic Sorry, I originally meant to remove the bodies in the second code snippet as they wouldn't make sense. Indeed, you cannot swap with a `const LinkedList&` (because it's `const`) - you'd have to construct a copy manually first (i.e. `LinkedList copy(other); swap(*this, copy); return *this;`), but that's better done by doing copy-and-swap the normal way. – Max Langhof Nov 26 '19 at 16:57
  • Didn't know about this trick. I use `operator=(other)` in implementing the move constructor though. – Tanveer Badar Nov 26 '19 at 17:02
  • @TanveerBadar If you already have `swap` for your type, you can just implement the move constructor as default-constructor + `swap(*this, other)` (but maybe there's a more efficient implementation - default-constructing and then swapping some plain `int` value is wasted effort, you might as well copy it in the first place). – Max Langhof Nov 26 '19 at 17:10
  • @MaxLanghof: if i looks in the link above (copy/swap paradime), i see this codesnip, which was actually what i used: `dumb_array& operator=(dumb_array other) // (1) { swap(*this, other); // (2) return *this; }` – tannic Nov 26 '19 at 17:50
  • @tannic Yes, when you do it that way you simply don't need an `LinkedList&&` overload for `operator=` anymore. Is there still something unclear? – Max Langhof Nov 27 '19 at 08:29
  • @MaxLanghof: ah of course. I didnt read it properly. The copy and move assignment operator would be the same, right ? – tannic Nov 27 '19 at 16:15