Problem with Comparator interface

Newbie & Debugging Questions
#1

I have a class called Module, which often needs to be sorted by priority. I therefore make it implement the Comparator interface, thus :


public int compareTo (Module d) {
      System.out.println("Comparing better");
      if (priority > d.priority) return -1;
      if (priority < d.priority) return 1;
      return 0;
}


public int compareTo (Object d) {
      System.out.println("Comparing boring");
      return 0;
}

Unhappily, the prints indicate that it is the second, default method that is being called when I ask for sorting of an array of subclasses of Module. I didn’t have this problem before I created the subclasses, so I’m reasonably sure this is the cause.

I tried making a compareTo method for each subclass, but it is still the generic method that is called. Does anyone have a good solution?

#2

are both the methods above implemented in module ? Try just implementing the method that takes Module as an argument, and not the one that takes Object. A subclass will pick up the most general method first that matches the method signature its looking for. So you can remove the second one entirely, or have the subclass also override the method.

D.

#3

Your first method with never be called by a routine that uses a Comparator. If you look at the Comparable interface, you will see that the method signature is the same as your second method. This is the one that will always be called. So if you have a super class that implements the Comparable interface, then all you have to do is have your subclasses override the compareTo(Object) method(as Daire said).

#4

oh, and thats the Comparable interface, not the Comparator interface. I presume it was just a typo on your part, as you would have got compile time errors instead of bugs :slight_smile:

Also a bit of a brainmush on my part, I’d forgotten you couldn’t override interface declarations like that. As CaptainJester said, you gotta use the same method declaration as in the interface, then cast to your particular circumstance, so your code will look something like this:


public int compareTo (Object d) 
{
    Module m = (Module)d;

    System.out.println("Comparing better");
    if (priority > m.priority) 
        return -1;
    else if (priority < m.priority) 
        return 1;
    else
        return 0;
}
#5

Comparable, right you are. I found the thing that was confusing me : My testing algorithm was a bit flawed, so it looked like it was working before I introduced the subclasses. In fact it wasn’t. :slight_smile:

Thanks for the replies. I’m not happy with the casting, though; I like to avoid that when I can. :frowning:

#6

Daire’s method is a bit off. Here is what you should do.


public int compareTo (Object d)  { 
  int result = 0;

  if(d instanceof Module) {
    Module that = (Module)d; 
 
  if (this.priority > that.priority)  {
    result = -1; 
  }
  else if (this.priority < that.priority)  
    result = 1; 
  }
  else {
     result = 0; 
  }

  return result;
}

You should first check to make sure that the Object is of the right type. Then you can safely cast it.

#7

oh right, did we decide eventually that we didn’t like multiple returns in the one method ?? :slight_smile:

To be honest, though I suppose the instanceof is neccessary for robustness sakes, I wouldn’t normally include it for a comparable implemented class. You’re very rarely in the situation where some collection is going to be filled with a bunch of objects that aren’t extended from a particular sub object, or all implement a particular interface.

D.

#8

Nah, that’s just the way I like to do it. ;D

Yeah, the chance is minimal, but you should always program for robustness from the start. It is easier to than spotting a bug.

#9

If the parameter passed in isn’t an instance of Module you should really throw an IllegalArgumentException. There is no point in sorting a list of objects only to find that in the middle of the list is a group of objects that you aren’t interested in.

This problem kind of goes away when you start to use the following code in Java 5.0

List myList = new ArrayList();

Andy.