Trying to convert an ArrayList to a LinkedList. Not sure why this does not work

Question

EDIT: Not sure why but the code seems to be working now without any edits. Could have been a problem with the jGrasp debugger?

===

Ok. So this is my homework assignment that will be assigned 2 weeks from now, but I want a head start. Please do not correct my code, or share the correct code. If you could pin-point the error in what I'm doing, that would be great.

So I have a node with following constructors:

public node(String name)
public node(String name, node next)

I need to write a method public method(ArrayList<String> names)in a separate class that will add all elements from names into the linked list.

Here's what I have right now:

public method(ArrayList<String> names) {
    if(names.size() == 0 || names == null) {
        throw new IllegalArgumentException();
    }

    // Handle base case, create first node
    first = new node(names.get(0));    // first has been declared above

    node current = first;

    // Add at the end of the list
    for(int i = 1; i < names.size(); i++) {
        current.next = new node(names.get(i));
        current = current.next;
    }

}

I'm not sure why this doesn't work as required. I'm using jGrasp, and using the debugger, I see that at the end, I get a linked list of just 1 value (the last element in the ArrayList). Why?

Please do not recommend using any advanced features, since I'm new to Java and using any further advanced features will simply confuse me.

The variable first should hold the whole linked list. Make sure you're not just returning current, which is the end of the linked list. — Mick, Jan 20 '13 at 05:01
How are you testing that your linked list contains a single value, using a property (that's not updated in your code above) or by iterating all the items from `first` until `current.next == null`? If you're using the last way, then please show the code you use in this method. And please, be careful with the code you post, it should at least compile. — Luiggi Mendoza, Jan 20 '13 at 05:01
Also, unrelated, but your `names.size() == 0 || names == null` check is backwards. If names is null, the first part of that check will throw an NPE. — Jeff Storey, Jan 20 '13 at 05:02
please show some more code? we donot see where you are adding current node to the linkedlist. +1 for the fact that you do not want readymade solution — vishal_aim, Jan 20 '13 at 05:04
@vishal_aim `current.next = new node(names.get(i)); current = current.next;` inside the `for-loop`. — Luiggi Mendoza, Jan 20 '13 at 05:05
@LuiggiMendoza that is just a node with all further nodes but where is that even single node added to the list — vishal_aim, Jan 20 '13 at 05:09
@vishal_aim `node current = first;` before starting the `for-loop`, now do you understand it? — Luiggi Mendoza, Jan 20 '13 at 05:10
@LuiggiMendoza yes but the current reference keeps changing the pointing object of node, if you are returning the last reference it'll contain only last node, it needs to make sure that the first one is returned — vishal_aim, Jan 20 '13 at 05:13
@vishal_aim test the code provided in my answer. OP's problem is not in the code posted, is somewhere else. — Luiggi Mendoza, Jan 20 '13 at 05:22

score 0 · Answer 1 · edited May 23 '17 at 12:21

I made a test using your code (and using JavaBean standard naming) and your method works fine. Here's the code sample (some long code block here):

import java.util.ArrayList;

class Node {
    private String data;
    private Node next;

    public Node(String data) {
        this.data = data;
        this.next = null;
    }

    public Node(String data, Node next) {
        this.data = data;
        this.next = next;
    }

    public String getData() {
        return data;
    }

    public Node getNext() {
        return next;
    }

    public void setNext(Node next) {
        this.next = next;
    }
}

public class NodeTest {

    private Node first;

    public NodeTest() {
        this.first = null;
    }

    //hint: this is your code, no changes were made here except by the method name
    public void insertArrayList(ArrayList<String> names) {
        //changing the order of the comparison. Java evaluates from left to right
        if(names == null || names.size() == 0) {
            throw new IllegalArgumentException();
        }

        // Handle base case, create first node
        first = new Node(names.get(0));    // first has been declared above

        Node current = first;

        // Add at the end of the list
        for(int i = 1; i < names.size(); i++) {
            current.setNext(new Node(names.get(i)));
            current = current.getNext();
        }
    }

    public void traverse() {
        Node current = first;
        while (current != null) {
            System.out.println(current.getData());
            current = current.getNext();
        }
    }

    public static void main(String[] args) {
        ArrayList<String> names = new ArrayList<>();
        names.add("Hello");
        names.add("world!");
        NodeTest nodeTest = new NodeTest();
        nodeTest.insertArrayList(names);
        nodeTest.traverse();
    }
}

Result:

Hello
world!

So, as posted in previous comments, maybe there's a problem about how you're testing if your linked list has been filled or you have a problem somewhere else in the non-showed code.

score 0 · Answer 2 · answered Jan 20 '13 at 05:23

I think you are returning the last node from the method while you need to return the first one as it contains all further linked nodes. You should return the first node not the current node.

If still you have issues, please show us how you are testing it to conclude that it contains only last element.

Trying to convert an ArrayList to a LinkedList. Not sure why this does not work

2 Answers2