Move Assignment operator c++

Question

I cannot tell what is wrong with my move assignment operator, here is the function. I don't think I am grabbing the data correctly, because when I run the test I get a random negative number and a "you're program has stopped working)

virtual LinkedList<T> &operator=(LinkedList<T> &&other)
{
    cout << " [x] Move *assignment* operator called. " << endl;

    // Delete our own elements
    ListNode<T> *temp1 = _front;
    while (temp1 != nullptr)
    {
        ListNode<T> *n = temp1->getNext();
        delete temp1;           
        temp1 = n;
    }
    // Grab other data for ourselves
    ListNode<T> *temp2 = other._front;
    while (temp2 != nullptr)
    {
        addElement(temp2->getValue());
        temp2 = temp2->getNext();
    }
    // Reset their pointers to nullptr

    other._front = nullptr;
    other._end = nullptr;
    other._size = 0;
    other._last_accessed_index = 0;
    other._last_accessed_node = nullptr;

    return *this;
}

Test Code- this is my teachers test code -

// Use move *assignment* operator
cout << " [x] Test #5: Move *assignment* constructor behavior" << endl;
moved1 = LinkedList<int>{ 6, 7, 8, 9, 10 };
cout << "   [x] Result:" << endl;
cout << "   [x]  Expected:\t6 7 8 9 10" << endl;
cout << "   [x]  Actual:\t\t";
for (int i = 0; i < moved1.getSize(); i++)
{
    cout << moved1.getElementAt(i) << " ";
}
cout << endl << endl;

this is my first time working with move and the move assignment operator. Thanks :)

Answer 1

This is not a proper implementation of a move assignment operator. It looks more like a copy assignment operator (but not a good one, as it leaks memory).

A typical move assignment operator would look more like this instead:

#include <utility>

LinkedList<T>& operator=(LinkedList<T> &&other)
{
    cout << " [x] Move *assignment* operator called. " << endl;

    std::swap(_front, other._front);
    std::swap(_end, other._end);
    std::swap(_size, other._size);
    std::swap(_last_accessed_index, other._last_accessed_index);
    std::swap(_last_accessed_node, other._last_accessed_node);

    return *this;
}

A move assignment operator should not free anything. Move ownership of the source's content to the target object, and vice versa. Let the source object free the target object's previous content when the source object is destroyed after the assignment operator exits, so make sure the class also has a proper destructor implementation:

~LinkedList()
{
    // Delete our elements
    ListNode<T> *temp = _front;
    while (temp != nullptr)
    {
        ListNode<T> *n = temp->getNext();
        delete temp;           
        temp = n;
    }
}

For good measure, here is what the copy constructor, move constructor, and copy assignment operators could look like:

LinkedList() :
    _front(nullptr),
    _end(nullptr),
    _size(0),
    _last_accessed_index(0),
    _last_accessed_node(nullptr)
{
    cout << " [x] Default *constructor* called. " << endl;
}

LinkedList(const LinkedList<T> &src)
    : LinkedList()
{
    cout << " [x] Copy *constructor* called. " << endl;

    ListNode<T> *temp = src._front;
    while (temp != nullptr)
    {
        addElement(temp->getValue());
        temp = temp->getNext();
    }
}

LinkedList(LinkedList<T> &&src)
    : LinkedList()
{
    cout << " [x] Move *constructor* called. " << endl;    
    src.swap(*this);
}

LinkedList(initializer_list<T> src)
    : LinkedList()
{
    cout << " [x] Initialization *constructor* called. " << endl;

    const T *temp = src.begin();
    while (temp != src.end())
    {
        addElement(*temp);
        ++temp;
    }
}

LinkedList<T>& operator=(const LinkedList<T> &other)
{
    cout << " [x] Copy *assignment* operator called. " << endl;

    if (&other != this)
        LinkedList<T>(other).swap(*this);

    return *this;
}

LinkedList<T>& operator=(LinkedList<T> &&other)
{
    cout << " [x] Move *assignment* operator called. " << endl;
    other.swap(*this);        
    return *this;
}

void swap(LinkedList<T> &other)
{
    std::swap(_front, other._front);
    std::swap(_end, other._end);
    std::swap(_size, other._size);
    std::swap(_last_accessed_index, other._last_accessed_index);
    std::swap(_last_accessed_node, other._last_accessed_node);
}

The copy and move assignment operators can actually be merged together into a single implementation, by taking the input object by value and letting the compiler decide whether to use copy or move semantics when initializing that object, based on the context in which the operator is called:

LinkedList<T>& operator=(LinkedList<T> other)
{
    cout << " [x] *assignment* operator called. " << endl;
    swap(other);
    return *this;
}