Does this assigment operator for linked list makes deep copy?

Question

is this assigment operator for linked list do deep copying? ///class my list have attribite named node* head;

MyList<type> &operator = ( MyList<type>  another_list )
    {
        node<type>* newNode= new node<type>();
        node<type>* head2= new node<type>();
        node<type>* cur = head;
        bool check = true;
        while(cur!=NULL)
        {
            newNode->data=cur->data;
            node<type>* newNode2= new node<type>();
            newNode->next=newNode2;
            if(check==true)
                {
                    head2=newNode;
                    check=false;
                }
            newNode=newNode->next;
            cur = cur->next;
        }
        another_list.head=head2;
        return another_list;
    }

We can't tell without seeing the `node` class. For example, we need to know what type `data` is. Is it a pointer? A reference? A value? — David Schwartz, Apr 09 '19 at 01:02
Assuming `MyList` implements a *proper* copy constructor (which we can't see), then `another_list` should ALREADY be deep-copied by virtue of being *passed by value*, and so there is no need to make *yet another* copy, just take ownership of what `another_list` has already copied. — Remy Lebeau, Apr 09 '19 at 01:05
is there a reason why you are not using `std::list` or `std::forward_list` instead? Let the C++ standard library do the heavy work for you. — Remy Lebeau, Apr 09 '19 at 01:32

Remy Lebeau · Answer 1 · 2019-04-09T01:28:10.890

What you have shown is not a properly implemented assignment operator, not even close. Your logic is backwards, you need to populate this with copies from another_list, not the other way around like you are trying to do.

It should look more like this instead:

MyList<type>& operator=(const MyList<type> &another_list)
{
    if (&another_list != this)
    {
        node<type> *cur = head;
        node<type> *next;

        head = NULL;

        while (cur != NULL)
        {
            next = cur->next;
            delete cur;
            cur = next;
        }

        node<type>** newNode = &head;

        cur = another_list.head;
        while (cur != NULL)
        {
            *newNode = new node<type>();
            (*newNode)->data = cur->data;
            (*newNode)->next = NULL;

            newNode = &((*newNode)->next);
            cur = cur->next;
        }
    }

    return *this;
}

Or better, using the copy-swap idiom:

// default constructor
MyList()
    : head(NULL)
{
}

// copy constructor
MyList(const MyList<type> &another_list)
    : head(NULL)
{
    node<type> *cur = another_list.head;
    node<type>** newNode = &head;

    while (cur != NULL)
    {
        *newNode = new node<type>();
        (*newNode)->data = cur->data;
        (*newNode)->next = NULL;

        newNode = &((*newNode)->next);
        cur = cur->next;
    }
}

// move constructor, C++11 and later only...
MyList(MyList<type> &&another_list)
    : head(another_list.head)
{
    another_list.head = NULL;
}

// destructor
~MyList()
{
    node<type> *cur = head;
    node<type> *next;

    while (cur != NULL)
    {
        next = cur->next;
        delete cur;
        cur = next;
    }
}

// copy assignment operator
MyList<type>& operator=(const MyList<type> &another_list)
{
    if (&another_list != this)
    {
        MyList<type> temp(another_list);
        std::swap(temp.head, head);
    }

    return *this;
}

// move assignment operator, C++11 and later only...
MyList<type>& operator=(MyList<type> &&another_list)
{
    MyList<type> temp(std::move(another_list));
    std::swap(temp.head, head);
    return *this;
}

If you don't need to support pre-C++11 compilers, you can merge the copy assignment and move assignment operators together into a single operator that is smart enough to invoke the copy constructor or move constructor depending on whether an lvalue or rvalue is being assigned:

// copy+move assignment operator
MyList<type>& operator=(MyList<type> another_list) // <-- pass by value!
{
    MyList<type> temp(std::move(another_list));
    std::swap(temp.head, head);
    return *this;
}

Does this assigment operator for linked list makes deep copy?

1 Answers1