Binary Tree Operations

I've been programming for about 5 years now, mostly in C++ and Java, while continuing my college education in Computer Science. As an adult student, I'm always looking for opinions and ways to improve from professionals in the field. Aside from reading from numerous text books, I've read these forums for tips and good programming practices to improve.

I'm posting a binary search tree in java to hear any and all feedback to continue with this trend of learning; any positive and negative criticism would be helpful to improve. Thanks in advance.

```
public class NewBinaryTree {

Node root; // top most node of tree

public void addNode(int key, String name) { // adds node to tree
Node newNode = new Node(key, name);

if(root == null) {
root = newNode;
}
else {
Node focusNode = root;
Node parent;

while(true) {
parent = focusNode;

if(key < focusNode.key){ // checks if the key is less than, then inserts left
focusNode = focusNode.leftChild;

if(focusNode == null) {
parent.leftChild = newNode;
return;
}
}
else { // key is greater than, so inserts on right
focusNode = focusNode.rightChild;

if(focusNode == null) {
parent.rightChild = newNode;
return;
}
}
}
}
}

public void inOrderTraverseTree(Node focusNode) { // in order tree traversal using recursion
if(focusNode != null) { // checks if something is in there
inOrderTraverseTree(focusNode.leftChild);
System.out.println(focusNode);
inOrderTraverseTree(focusNode.rightChild);
}
} // end in order tree traversal

public void preOrderTraverseTree(Node focusNode

Many of your comments are superfluous. If you flood your reader with useless comments, they might skip some important ones:

// adds node to tree - this is already clear from method name - addNode.

// end in order tree traversal - markers such as this are unnecessary, if you are using modern IDE. In most editors there are much better ways to easily navigate through your code.

getRePlacementNode should be private. It doesn't look it would be useful to users of your class. This way you will be able to change it without breaking backwards compatibility.

remove(int) is really long. It looks like it could be split up into smaller methods. You should re-use findNode(int) instead of copying it.

I'm not sure if your program works as intended, try:

theTree.addNode(0, "a");
theTree.addNode(1, "b");
theTree.addNode(2, "c");
theTree.remove(1);
theTree.inOrderTraverseTree(theTree.root);

should both 1 and 2 be removed? It's a good idea to add unit tests to your classes. This way, if there is a test checking this behavior, it probably is intended. If not, it's probably a bug.

StackExchange Code Review Q#27412, answer score: 4