My own version of a search binary tree

I wrote my own version of a search binary tree in Java. Could someone kindly review it?

```
public class Tree {

private Node root;

public Node find(int id) {

Node current = root;

while(current != null) {

if (current.getId() == id)

// Node found
return current;

current = goNextChild(current, id);
}

return current;
}

public void insert(int id, String name) {

Node node = new Node(id, name);

if (root == null)
root = node;
else {

Node current = root;
while(current != null) {

if (current.getId() == id) {
// If present, just change information of the current node
current.setId(id);
current.setName(name);
return;
}
else if (current.getId() id) {
// go to the left child
if (current.getLeftChild() == null) {
// If the left child is not present set the new node
current.setLeftChild(node);
return;
}
else {
current = current.getLeftChild();
}
}
}
}
}

public void delete(int id) {

Node delNode = find(id);
Node parent = findParent(delNode);

if((delNode.getLeftChild() == null)
&& (delNode.getRightChild() == null)) {

// Delete node with no children

if (delNode == root)
root = null;
else if(parent.getLeftChild() == delNode)
parent.setLeftChild(null);
else if(parent.getRightChild() == delNode)
parent.setRightChild(null);
}
else if(delNode.getLeftChild() == null) {

// Delete node

Design

• I highly recommend that you create a Tree interface that can be implemented by a BinarySearchTree class. Take your insert, delete, find, etc. methods and declare them over there. Suppose you decide to create a Red-Black tree, then you could create a RedBlackTree class which implements the same interface. This makes swapping out implementations very easy! Here's a good link to read: Why would a programmer want to separate implementation from interface?

• Why do you need the TreeData class? If you ask yourself "What is a node in a tree and what does it do?", then your POJO should reflect that answer. In this case, a tree node has a key, a value (or some piece of data), a left child, and a right child. Why shouldn't the Node represent this itself? Sure, you don't need to expose setter methods for each attribute, but by adding getter methods on Node, you're still exposing the attributes indirectly.

Correctness

• Take a look at your find() method. Currently, if the Node you're searching for doesn't exist, you return the root of the tree. Is that really what you want to do? I would either return null, or take a look at the Optional documentation (would recommend going this route as it's a good way to help avoid null pointer exceptions!).

Minor Notes

• Make variables final when possible. It'll make your code less error-prone and is just general good-practice.

• Take a look at your 'if-else' statements, and you'll notice you've got some cases where you have 'if, else if, else if'. I highly recommend following a conventional 'if, else if, else' pattern to prevent bugs in the code. If you need to, add some documentation to clarify the branches.

StackExchange Code Review Q#103801, answer score: 4