NullPointerException when adding node objects to an ArrayList

Question

I'm getting a NullPointerException when I try to run this code. I've assigned Nodes 2,3,and 4 as children nodes to Node1. I tried to create a method that will go through all the children nodes in Node1 and return the list. I'm not sure what I'm doing wrong.

public class TestingArrays2 {

    List<Node> myList1;
    List<Node> myList2;
    List<Node> myList3;
    List<Node> myList4;

    private Node Node1;
    private Node Node2;
    private Node Node3;
    private Node Node4;

    public TestingArrays2() {
        myList1 = new ArrayList<Node>();
        myList2 = new ArrayList<Node>();
        myList3 = new ArrayList<Node>();
        myList4 = new ArrayList<Node>();

        myList1.add(Node2);
        myList1.add(Node3);
        myList1.add(Node4);

        Node1 = new Node("One", myList1);
        Node2 = new Node("Two", myList2);
        Node3 = new Node("Three", myList3);
        Node4 = new Node("Four", myList4);

        List<Node> allNodesArray = nodeArray(Node1);

        for (int i = 0; i < allNodesArray.size(); i++){
            System.out.println(allNodesArray.get(i).label);
        }
    }

    public static void main(String arg[]) {
        TestingArrays2 testArray = new TestingArrays2();
    }

    public List<Node> nodeArray(Node n){
        List<Node> tempList = new ArrayList<Node>();

        for (int i = 0; i < n.children.size(); i++){
            tempList.add(n.children.get(i));
        }

    return tempList;
    }
}

Answer 1

You're not creating your Nodes. See these lines...

private Node Node1;
private Node Node2;
private Node Node3;
private Node Node4;

These just declare a variable as being able to contain an object of type Node. However, they initially start with a null value - ie they're empty.

You're then calling these lines...

myList1.add(Node2);
myList1.add(Node3);
myList1.add(Node4);

Which would insert null values into your List, because you're trying to add an object that hasn't been created yet.

So, you need to change your code so that these lines...

Node1 = new Node("One", myList1);
Node2 = new Node("Two", myList2);
Node3 = new Node("Three", myList3);
Node4 = new Node("Four", myList4);

appear before you try to myList1.add() them to the list. This will create the Node objects first, which can then be added to your List.

As @BalusC mentioned in the comments, it is failing on your for loop later in your code, because it is trying to call .label on a null object. Correcting the order as suggested above will correct this, as all the objects in your List will now be Nodes.

Answer 2

A good answer to your question is already given.

Looking at your code I have several suggested modifications.

You are doing all the work in (the constructor of) your test class. It is nicer design to delegate this to the Node class where possible. Also try not to do 'work' in the constructor, just initialization.

Also check out the code conventions I applied like using nouns for class names and starting variable names with a lower case letter.

public class ArrayTest2 {

    public static void main(String arg[]) {

        Node node1 = new Node("One");

        node1.add(new Node("Two"));
        node1.add(new Node("Three"));
        node1.add(new Node("Four"));

        // this calls the toString method of node1
        System.out.println(node1);
    }

}

public class Node {

    private final String name;
    private final List<Node> children;

    public Node(String name) {
        this.name = name;
        this.children = new ArrayList<Node>();
    }

    public String getName() {
        return name;
    }

    public void add(Node children) {
        children.add(child);
    }

    @Override
    public String toString() {
        StringBuilder sb = new StringBuilder();
        sb.append(name);
        if(children.size() > 0) {
            sb.append("(");
            String separator = "";
            for (Node child : children){
                sb.append(separator).append(String.valueOf(child));
                separator = ",";
            }
            sb.append(")");
        }
        return sb.toString();
    }
}

Notice that the children field is private and there is no getter for it. It is considered bad practice to offer direct access to internal data structures like 'children' so I provided an 'add' method for adding nodes. In this way the class keeps control over what happens to its data, which is an important OO design principle.

The toString method builds a string representation of a Node. It appends the Node name and then, in case there are children, appends each child node's string representation in a comma-separated list surrounded by parentheses, so this should print something like:

One(Two,Three,Four)

A more complex structure for example:

Node node1 = new Node("One");
Node node2 = new Node("Two");
Node node3 = new Node("Three");
Node node4 = new Node("Four");
Node node5 = new Node("Five");
Node node6 = new Node("Six");
node1.add(node2);
node1.add(node3);
node2.add(node4);
node4.add(node5);
node4.add(node6);

Should give:

One(Two(Four(Five,Six)),Three)

Disclaimer: my code is hand-crafted, uncompiled and untested

Answer 3

This:

myList1.add(Node2);
myList1.add(Node3);
myList1.add(Node4);

Node1 = new Node("One", myList1);
Node2 = new Node("Two", myList2);
Node3 = new Node("Three", myList3);
Node4 = new Node("Four", myList4);

You are trying to add the nodes to the list before they have been created.