Copy function Linked list c++

Question

I'm trying to write a function to copy the contents of one linked list into a new linked list (no reference to the first LL). I've so far got this:

void List::copy(const List& otherList)
{
    assert(head == nullptr);
        if (otherList.head != nullptr)
        {
            head = new Node;
            assert(head != nullptr);
            head->item = otherList.head->item;
            Node* ptr1 = head;
            for (Node* ptr2 = otherList.head->next; ptr2 != nullptr; ptr2=ptr2->next)
            {
                ptr1->next = new Node;
                assert(ptr1->next != nullptr);
                (ptr1->next)->item = ptr2->item;
                (ptr1->next)->next = ptr2-> next;
            }
        }
}

However when I run the code on a small linked list, it simply copys the first and last nodes - for some reason it misses out the middle part. I've spent a while researching other peoples solutions and trying to figure out whats going wrong with mine, however i've hit a brick wall!

Could someone possibly point out where i'm going wrong?

Kind regards

Craig

You don't move ptr1 (in other words, it always points to head). Thus you get the head and tail into your new list. — crashmstr
– crashmstr, Commented Dec 10, 2014 at 15:52

George Houpis · Accepted Answer · 2014-12-10 15:54:27Z

2

You are not iterating down you new list.

void List::copy(const List& otherList)
{
    assert(head == nullptr);
        if (otherList.head != nullptr)
        {
            head = new Node;
            assert(head != nullptr);
            head->item = otherList.head->item;
            Node* ptr1 = head;
            for (Node* ptr2 = otherList.head->next; ptr2 != nullptr; ptr2=ptr2->next)
            {
                ptr1->next = new Node;
                assert(ptr1->next != nullptr);
                (ptr1->next)->item = ptr2->item;
                ptr1 = ptr1->next;
            }
            ptr1->next = 0;
        }
}

answered Dec 10, 2014 at 15:54

George Houpis

1,7291 gold badge9 silver badges5 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

jez · Accepted Answer · 2014-12-10 15:52:58Z

1

Your code never updates the value of ptr1. It stays equal to head and you constantly update the new head's ->next, leaking allocated Nodes into limbo.

answered Dec 10, 2014 at 15:52

jez

15.5k6 gold badges43 silver badges77 bronze badges

Comments

Jonathan Mee · Accepted Answer · 2014-12-10 15:55:45Z

0

Change this:

Node* ptr1 = head;
for (Node* ptr2 = otherList.head->next; ptr2 != nullptr; ptr2=ptr2->next)
{
    ptr1->next = new Node;
    assert(ptr1->next != nullptr);
    (ptr1->next)->item = ptr2->item;
    (ptr1->next)->next = ptr2-> next;
}

To this:

for (Node* ptr2 = otherList.head->next, ptr1 = head; ptr2 != nullptr; ptr2 = ptr2->next, ptr1 = ptr1->next)
{
    ptr1->next = new Node;
    (ptr1->next)->item = ptr2->item;
    (ptr1->next)->next = nullptr;
}

This change will initialize both of your iterators only in the for loop, and will advance them in lockstep.

answered Dec 10, 2014 at 15:55

Jonathan Mee

39.1k26 gold badges148 silver badges320 bronze badges

Collectives™ on Stack Overflow

Copy function Linked list c++

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related