HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavaMinor

Create a binary tree, code review request

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
createrequestbinarycodereviewtree

Problem

Ok, code reviewers, I want you to pick my code apart and give me some feedback on how I could make it better or more simple. ( Generics would be added a bit later).

public class CreateABinaryTree {
    private TreeNode root;

    public CreateABinaryTree() {
    }

    /**
     * Constructs a binary tree in order of elements in an array.
     * After the number of nodes in the level have maxed, the next 
     * element in the array would be a child of leftmost node.
     * 
     * http://codereview.stackexchange.com/questions/31334/least-common-ancestor-for-binary-search-tree/31394?noredirect=1#comment51044_31394
     */
    public CreateABinaryTree(List items) {
        this();
        create(items);
    }

    private static class TreeNode {
        TreeNode left;
        int element;
        TreeNode right;

        TreeNode(TreeNode left, int element, TreeNode right) {
            this.left = left;
            this.element = element;
            this.right = right;
        }
    }

    private void create (List items) {   
        root = new TreeNode(null, items.get(0), null);

        final Queue queue = new LinkedList();
        queue.add(root);

        final int half = items.size() / 2;

        for (int i = 0; i < half; i++) {
            if (items.get(i) != null) {
                final TreeNode current = queue.poll();
                final int left = 2 * i + 1;
                final int right = 2 * i + 2;

                if (items.get(left) != null) {
                    current.left = new TreeNode(null, items.get(left), null);
                    queue.add(current.left);
                }
                if (right < items.size() && items.get(right) != null) {
                    current.right = new TreeNode(null, items.get(right), null);
                    queue.add(current.right);
                }
            }
        }
    }
}

Solution

I only see one really questionable thing in your code. Why do you call this() in your constructor? Your parameter-less constructor does absolutely nothing, so this line is irrelevant. If anything, I would actually even remove that constructor entirely so that your API can only be used with your meaningful and useful one.

Also, as a side note, the following line opens you up to a NullPointerException since you never check to see if items is null or if it has any elements.

root = new TreeNode(null, items.get(0), null);

Code Snippets

root = new TreeNode(null, items.get(0), null);

Context

StackExchange Code Review Q#31998, answer score: 4

Revisions (0)

No revisions yet.