Confused on how to do a deep copy of a doubly Linked List?

Question

I have been trying to implement a deep copy of a doubly linked list, but I have been having trouble with it. I have to do it different several times, but I ended up getting an address error. I just need an explanation how to do it properly.

List.H

class List 
{
public:
    List();
    ~List();
    List(const List& c);
    List& operator= (const List& t);
private:
    List *Next;
    List *Prev;
    Node *Head;

List.cpp

List::~List()
{
    Node* move = Head;
    while (move!=NULL)
    {
        Node *temp = move->Next;
        delete move;
        move = temp;
    }
}

List::List(const List& c)
{
    name = c.name;
    if (c.Head == NULL) {
        Head = NULL;
    }
    else {
        Head = new Node(*c.Head);
        Node* Current = Head;
        Node* ObjHead = c.Head;
        Node* CurrentObj = ObjHead;

        while (Current->Next!=NULL) {
            Current->Next = new Node (CurrentObj->Next->condiments);
        }
    }
}

Seems like incorrect design to me, to begin with. Why do you have `List *prev, *next`? They should be `Node *prev, *next` instead. As it is, it is a linked list of lists, not nodes. — Tanveer Badar, Jan 18 '20 at 06:11
Well i have two doubly linked list. Node manages a doubly linked list of data. Then List manages the nodes. — Jay, Jan 18 '20 at 06:15

score 2 · Answer 1 · answered Jan 18 '20 at 07:08

2

Copying linked lists is about three things:

Traversing the list being copied.
Making copies of new nodes from the originals.
For each new node from (2), tail-link it to the linked list.

The first of these is trivial, the second is fairly basic, but the third is the one that often tosses people for a loop. For your copy-ctor, one way to do it employs a pointer-to-pointer. This allows us to address each pointer in our linked list by their own addresses.

List::List(const List& c)
    : Head(nullptr)
    , name(c.name)
{
    Node *prev = nullptr;
    Node **pp = &Head;

    for (const Node *p = c.Head; p; p = p->Next)
    {
        // make a new node, storing its address at the
        // pointer obtained by dereference of `pp`. the
        // first iteration that will be the Head member.
        *pp = new Node(*p);
        (*pp)->Prev = prev;
        prev = *pp;

        // now just advance `pp` to point to the `Next`
        // member of the node we just hung on the list.
        pp = &(*pp)->Next; 
    }
    *pp = nullptr; // terminate the list.
}

This assumes you're Node class supports copy-construction (it had better). but that's all it takes. From that, you can use the copy/swap idiom to manufacture your copy-assignment operator and have a basic rule-of-three compliance list class.

answered Jan 18 '20 at 07:08

WhozCraig

59,130
9
69
128

my copy constructor is working fine now but, the copy assignment is where im having some issues. Im trying to do it without the copy/swap method. – Jay Jan 18 '20 at 07:28
1

Wouldn't it be much better to have an `Insert()` and call that from the copy constructor? – Tanveer Badar Jan 18 '20 at 07:35
1

@TanveerBadar no. With the only info I've been given about this list there is no tail pointer. `Insert` would therefore have to iterate-to-end with each insertion, and therefore turn a copy-ctor operation into O(N^2) rather than the *very* trivial O(N) seen above. if you want to get "better", it would be "better" to not do any of this madness and just use `std::list` in the first place. – WhozCraig Jan 18 '20 at 07:39
@WhozCraig Yeah im sure its easy to do with std::List but i need to implement this without it or any other container. So to my understanding I need to also traverse through the list in the copy assignment operator also. – Jay Jan 18 '20 at 07:56
@Jay, you need to implement one, yes. You don't need to necessarily do this sort of thing. [See the link](https://stackoverflow.com/a/3279550/1322972) in my answer. – WhozCraig Jan 18 '20 at 18:20

score 0 · Answer 2 · answered Jan 18 '20 at 06:24

0

You can use a dummy head to begin with. Once the deep copying is done, you can set head to the next of the dummy head and delete the dummy head. You also won't have to check for if (c.Head == NULL) this way.

    Node *ptr1, *ptr2;
    head = ptr1 = new Node();
    ptr2 = c.head;
    while (ptr2)
    {
        ptr1->next = new Node(*ptr2);
        ptr1 = ptr1->next;
        ptr2 = ptr2->next;
    }

    Node *temp = head;
    head = head->next;
    delete temp;

answered Jan 18 '20 at 06:24

srt1104

929
7
10

@ lucieon when i run it and try to get the data from a node i get an exception thrown. read acess violation. *this* was nullptr. Not sure why thats happening – Jay Jan 18 '20 at 06:34
I don't see a case where this would try to read a `nullptr`. Try making the function `const` like this `List& operator= (const List& t) const;`. Also make sure you initialize the pointers to `nullptr` in the default constructor of `Node` class. – srt1104 Jan 18 '20 at 06:42
@luceion Yeah i made sure pointers were all set to nullptr. i pretty sure the issue is somewhere in the copy assignment. – Jay Jan 18 '20 at 06:51
Try running it through a debugger. – srt1104 Jan 18 '20 at 06:54

Confused on how to do a deep copy of a doubly Linked List?

2 Answers2