Transferring a String array to Binary Tree

Question

I'm trying to write a method that can transfer an array into a Binary tree. I know the code is not right, I run it and hope it would show something that I can continue to fix it. But it just kept loading without any error or result. May anyone give me some advice, please! Here is the BST class:

public class BSTNode {
    private String data;

    private BSTNode left;
    private BSTNode right;

    public BSTNode(String data) {
        this.data = data;
        this.right = null;
        this.left = null;
    }

    public String getData() {
        return data;
    }

    public void setData(String data) {
        this.data = data;
    }

    public BSTNode getLeft() {
        return left;
    }

    public void setLeft(BSTNode left) {
        this.left = left;
    }

    public BSTNode getRight() {
        return right;
    }

    public void setRight(BSTNode right) {
        this.right = right;
    }
}

And my method:

public BSTNode fromArray(String[] array, int start, int end) {
    int i = start;
    BSTNode root = new BSTNode(array[i]);
    BSTNode current = root;
    BSTNode parent = null;
    while (i <= end) {
        if (array[i].compareTo(current.getData()) < 0) {
            parent = current;
            current.setLeft(current); //Should I put current.setLeft(root) here?
        } else {
            parent = current;
            current.setRight(current);
        }
        // Create the new node and attach it to the parent node
        if (array[i].compareTo(parent.getData()) < 0) {
            parent.setLeft(new BSTNode(array[i]));
        } else {
            parent.setRight(new BSTNode(array[i]));
        }
        i++;
    }
    return current;
}

Thank you for your time!

Answer 1

After you've initialized the root, you've already inserted the first element, so you can increment i right away.

You set the left and right pointers without keeping track of the prior value.

You never change the value of current within the loop, and, as you immediately assign it to parent, you don't change parent either.

You should return the root instead of the current node.

How about something like this:

    while (++i <= end) {
        current = root;
        while (current != null) {
            parent = current;
            if (array[i].compareTo(current.getData()) < 0) {
                current = current.getLeft();
            } else {
                current = current.getRight();
            }
        }
        // Create the new node and attach it to the parent node
        if (array[i].compareTo(parent.getData()) < 0) {
            parent.setLeft(new BSTNode(array[i]));
        } else {
            parent.setRight(new BSTNode(array[i]));
        }
    }
    return root;

UPDATE:

You can avoid a redundant comparison by keeping its result in a variable:

    while (++i <= end) {
        boolean left;
        current = root;
        do {
            parent = current;
            left = array[i].compareTo(current.getData()) < 0;
            if (left) {
                current = current.getLeft();
            } else {
                current = current.getRight();
            }
        } while (current != null);
        // Create the new node and attach it to the parent node
        if (left) {
            parent.setLeft(new BSTNode(array[i]));
        } else {
            parent.setRight(new BSTNode(array[i]));
        }
    }
    return root;