Search code examples
javalinked-listcopy-constructor

How to properly copy a linked list using copy constructor in Java?


I can't properly use the copy constructor to make a copy of LinkedList.

Consider this example:

public class LinkedList {
    
    public class Node {
        Node next;
        int val;
        Node(int val) {
            this.val = val;
            next = null;
        }
    }
    public Node head;
    public Node current;
    public LinkedList() {
        
    }
    
    
    public LinkedList(LinkedList another) {
        this.head = another.head;
        this.current = another.current;
    }
    
    public boolean empty() {
        return head == null;
    }
    
    public void insert(int e) {
    
        if (empty()) {
            head = current = new Node(e);
            return;
        }
        Node currNode = new Node(e);
        Node tmp = current;
        tmp.next = currNode;
        current = currNode;
        

    }
    
    public void display() {
        Node currNode = head;
        while (currNode != null) {
            System.out.println(currNode.val);
            currNode = currNode.next;
        }
    }
}

I made a copy of linked list using copy constructor, code in main is below:

public class Main {
    public static void main(String [] args) {
        LinkedList ls = new LinkedList();
        ls.insert(5);
        ls.insert(6);
        ls.insert(7);
        
        LinkedList another = new LinkedList(ls);
        
        another.display();
        another.insert(0);
        ls.display(); // expected output is 5 6 7 
    }
}

The output of the code is 5 6 7 5 7 6 0, however I expected it to be 5 6 7 because I made copy, it won't affect ls. What's going in? How to fix that to get the expected output?


Solution

  • Something like

    public LinkedList(const LinkedList& another) {
        this->head = this->current = another.copy()
    }
    
    private LinkedList copy( const LinkedList& another ) {
        Node last_node = null;
        Node new_first = null;
        for ( Node next_node = another->head; next_node != null; 
              next_node = next_node->Next ) {
            Node new_node = new Node( next_node );
            if ( new_first == null )
                new_first = new_node;
            if ( last_node != null )
                last_node.Next = new_node
            last_node = next_node;
         }
         return new_first;
    }
    

    NOTE that I also changed the signature of the copy constructor; in general, the argument should be a const reference. Also, my simple solution contains a storage leak, since it isn't deleting the current nodes of "this" before copying from another.