Table with a Binary search tree

Question

**

Write the beginnings of a class for a Table implemented with a Binary Search Tree. (done) Then create a Table class with only one data member for the root. (done) Provide the following methods: a constructor that produces an empty table (done)
a method to insert a given item, (done) Create a separate class for a Node, with public data members (data, left, right) to support the Table class. (done) Use the following interface to declare the type of the data. interface Sortable{ public int compareTo( Sortable b );} (done) This does not have to be compiled and tested

** Yes, this is a hw assignment (I'm not asking for it to be done for me, it should already be done if I understood it correctly), I'm asking for a code review and any help with commenting on my code (if anything is wrong would be lovely to know (like if my logic is off in a section so I can go to it and address it). I don't have access to tutors as easily now so using this to hopefully check my work and research what I am lacking in understanding is my goal. Otherwise, my train of thought I hope is on the right path to understanding this. Thanks

package com.stackexchange.codereview;
interface Sortable {
 public int compareTo(Sortable other);
// a.compareTo(b) returns 0 if a == b, 
// -1 if a < b, and +1 if a > b
}
class TreeNode { // This is the separate class for node
 public Sortable _data; // this is the data variable
 public TreeNode left; // this stores left field
 public TreeNode right; // this stores right field
}
class Table {
 private TreeNode _root = null; // This is the table class with only data member head
 public void insert(Sortable item) { // This insert a new item to the table
 int compare = 0;
 TreeNode current = _root;
 TreeNode parent = null;
 while (current != null) {
 parent = current;
 compare = item.compareTo(current._data);
 if (compare == 0)
 return;
 if (compare < 0)
 current = current.left;
 else
 current = current.right;
 }
 TreeNode add = new TreeNode();
 add._data = item;
 add.left = add.right = null;
 if (parent == null)
 _root = add;
 else if (compare <= 0)
 parent.left = add;
 else
 parent.right = add;
 }
} 

• AJNeufeld: Doesn't look like this will compile. If shouldn't be capitalized. Sortabe is missing an L, and so on. Copy the code from Eclipse and paste it in the question body; don't try to transcribe it in by hand.
• blazed: Fixed the Sortable issue, and other mistakes created by putting it into a text editor first. I've been given conflicting information (some say use an ide to code, others say write it by hand first), was working on writing it by hand but that leads to other unforeseen issues, also thank you for looking it over.
• Maarten Bodewes: I've added indentation. See it as a code review remark: terrible indentation.
• blazed: Yes, I should have ctrl +I another mistake (is ctrl +i) an acceptable/normal way to indent or?

Answer 1

// a.compareTo(b) returns 0 if a == b

That's a JavaDoc description, so put it between /** and */ and before the method. We never document anything on the next line. Preferably comments are before a code line, and sometimes behind it.

class TreeNode { // This is the separate class for node

The class should probably be a static class within the Table class instead. We try and only use one class per file in Java (and only one public class is actually allowed). You are using public fields, but that's not a good idea; you can give them package access level (no modifier) as long as you don't expose such an implementation class.

public Sortable _data; // this is the data variable

In Java we never start any data, including fields, with an underscore. Besides that, it is no different than left and right.

private TreeNode _root = null; // This is the table class with only data member head

Wrong place to document Table, again, JavaDoc. Fields are assigned 0, 0.0 or indeed null by default, so there is no reason to do that explicitly.

int compare = 0;

This is a common mistake. The first thing you do with the variable is to assign it a value. In that case there is absolutely no need to assign it zero - and it might even hide errors later on.

if (compare == 0)
 return;

Always put code blocks between braces. Put an empty between the if and the next if so they don't look part of the same branching instruction.

TreeNode add = new TreeNode();

add is not a good name for a variable. First of all, it's a verb, and variables are not actions. Try nodeForItem.

add.left = add.right = null;

A maximum of one assignment per line should be used.

• blazed: 1. Noted on how to document comments. 2. noted on the use of public (typically we use private in class) that was my mistake. 3.The underscore thing is something my professor does (No idea why), it is somewhat of a habit at this point that I know is a bad one, noted. 4. Noted on the incorrect placement of table and my use of null. 5. That was an initialization error I had in eclipse that fixed it, but I see why that is a mistake. 6. my naming conventions were straight out of the notes he gave us so, that was lazy of me, apologies. I appreciate the review and will work on this more!