Alternate way for comparison call back function?

Question

I'm doing a programming practice problem and was wondering if someone can suggest a better to create/setup the call back for the comparison function. I have the following classes:

public class myTreeItem<T> {
 private T item;
 private myTreeItem<T> left;
 private myTreeItem<T> right;
 private myTreeItem<T> parent;
}

I have the usual get/set functions. This item go into a tree, whose definition is:

public class myTree<T> {
 myTreeItem<T> root;
 protected comparableCallBack<T> callback; // call back function for comparisons, to be provided by the creator of the tree
 public interface comparableCallBack<T> {
 int onCompare(myTreeItem<T> item1, myTreeItem<T> item2);
 }
}

I omitted all the other functions to help with readability. I am creating the tree in the following way:

 // create a regular tree
 myTree<Integer> tree1 = new myTree<Integer>();
 tree1.setCallback(new comparableCallBack<Integer>() {
 public int onCompare(myTreeItem<Integer> item1,
 myTreeItem<Integer> item2) {
 // Do the actual comparison here.
 if (item1.getItem().intValue() < item2.getItem().intValue())
 return -1;
 else if (item1.getItem().intValue() == item2.getItem().intValue())
 return 0;
 else
 return 1;
 }
 });

Are there any other options/ideas for the call back?

Answer 1

Comparator

There is no need for your interface.... what's wrong with Comparator? Why do you need to have your own?

The reason you appear to have it is because you need to compare the myTreeItem against another myTreeItem and the generic type is not right for you, but you can solve it easily with wrapping the Comparator in your myTree class when you need to....

So, your code has:

tree1.setCallback(new comparableCallBack<Integer>() {
 public int onCompare(myTreeItem<Integer> item1,
 myTreeItem<Integer> item2) {
 .......
 }
 });

but it should instead have:

tree1.setCallback(new Comparator<Integer>() {
 public int compare(Integer item1, Integer item2) {
 .......
 }
 });

and then, your library should have it's own private Comparator that looks something like:

class TreeComparator <T> implements Comparator<myTreeItem<T>> {
 private final Comparator<T> delegate;
 TreeComparator(Comparator<T> delegate) {
 this.delegate = delegate;
 }
 public int compare(myTreeItem<T> a, myTreeItem<T> b) {
 return delegate.compare(a.getItem(), b.getItem());
 }
}

Code Style

Read the Java Code-style guidelines.

Java class names should start with a capital letter.

• user3379755: Yes I was trying to create a comparator for the tree item. The suggestions make sense... I wasn't sure how to use the Comparator to create my own (guess I am more rusty than I thought). Agree on the guidelines too... I was trying to quickly put something together to post here I didn't follow all the guidelines.

Answer 2

I agree with @rolfl that Comparator should be used here, +1, and some other notes:

1. If doesn't make sense to create a tree without a comparator it should be a required constructor parameter. It would reduce the possible number of misuses (as well as bugs and NullPointerExceptions).

2. Just a sidenote: it's good to know that most Java types implements Comparable:

   public interface Comparable<T> {
    public int compareTo(T o);
   }
   

   You could use that it the type definition:

   public class myTree<T extends Comparable<T>> {
   

   Although, it's probably too strict. Not all types implements this interface and sometimes it's required to create a tree based on another comparison (for example, to build reversed tree).

3. The comparableCallBack interface could be static. See: Effective Java 2nd Edition, Item 22: Favor static member classes over nonstatic

4. public int onCompare(myTreeItem<Integer> item1,
    myTreeItem<Integer> item2) {
    // Do the actual comparison here.
    if (item1.getItem().intValue() < item2.getItem().intValue())
    return -1;
    else if (item1.getItem().intValue() == item2.getItem().intValue())
    return 0;
    else
    return 1;
    }
   

   You could extract out two local variables here to remove some duplication:

   int value1 = item1.getItem().intValue();
   int value2 = item2.getItem().intValue();
   

   (It looks like that the closing brace closes the if statement. It should be one level outer.)

5. If you don't have a good reason to not to do that, make the fields private:

   myTreeItem<T> root;
   protected comparableCallBack<T> callback; // call back function for comparisons, to be provided by the creator of the tree
   

   (Should I always use the private access modifier for class fields?; Item 13 of Effective Java 2nd Edition: Minimize the accessibility of classes and members.)

6. Comments like this are rather noise:

   // Do the actual comparison here.
   

   The code is obvious here, so I'd remove the comment. (Clean Code by Robert C. Martin: Chapter 4: Comments, Noise Comments)