2

I get the following error:

Comparison method violates its general contract!

This is my compareTo method

public int compareTo(ComparableItem another) {
     if (this.score == another.score)
        return this.getItemName().compareTo(another.getItemName());
     else if ((this.score) > another.score)
        return 1;
     else
        return -1;
}

I want to compare the scores of the items. But when the scores are the same I want to sort them by name.

What do I need to change and why do I get this error?

EDIT:

score is an int, itemname a String.

This is the ComparableItem class:

public abstract class ComparableItem extends MenuListItem implements Comparable<ComparableItem> {

protected int score;

public ComparableItem(String itemID, String itemName) {
    super(itemID, itemName);
}

public int compareTo(ComparableItem another) {
     if (this.score == another.score)
        return this.getItemName().compareTo(another.getItemName());
     else if ((this.score) > another.score)
        return 1;
     else
        return -1;
}

public abstract boolean found(String criteria);

And this is the MenuListItem class :

public class MenuListItem {

private String itemID,itemName;

public MenuListItem(String itemID, String itemName){
    setItemID(itemID);
    setItemName(itemName);
}

public String getItemID() {
    return itemID;
}

public void setItemID(String itemID) {
    this.itemID = itemID;
}

public String getItemName() {
    return itemName;
}

public void setItemName(String itemName) {
    this.itemName = itemName;
}

@Override
public String toString() {
    return this.itemName;
}
Robby Smet
  • 4,649
  • 8
  • 61
  • 104

1 Answers1

4

Quote from the Java API of Comparable<T>:

The natural ordering for a class C is said to be consistent with equals if and only if (e1.compareTo((Object)e2) == 0) has the same boolean value as e1.equals((Object)e2) for every e1 and e2 of class C.

And for using compareTo, the api further says:

It is strongly recommended (though not required) that natural orderings be consistent with equals. This is so because sorted sets (and sorted maps) without explicit comparators behave "strangely" when they are used with elements (or keys) whose natural ordering is inconsistent with equals. In particular, such a sorted set (or sorted map) violates the general contract for set (or map), which is defined in terms of the equals method.

So you should probably override equals to be consistent with your compareTo(), because otherwise two different objects e1 and e2 with equal score and getItemName() will have e1.compareTo((Object)e2) == 0) but not e1.equals((Object)e2).

DaveFar
  • 7,078
  • 4
  • 50
  • 90
  • It also says *"It is strongly recommended (**though not required**) that natural orderings be consistent with equals."*. But whatever is sending the exception might have a strict reading of that rule. – assylias Oct 23 '12 at 14:31
  • There are even [situations where you need a compareTo method that is inconsistent with equals](http://stackoverflow.com/a/12588005/829571). ;-) – assylias Oct 23 '12 at 14:34
  • How can I make my equals method consistent with compareTo? I've tried this : @Override public boolean equals(Object o) { final ComparableItem other = (ComparableItem) o; if (this.score == other.score && this.getItemName().equals(other.getItemName())) return true; else return false; } – Robby Smet Oct 23 '12 at 14:58
  • @just8laze that should suffice to align equals and compareTo – Alex Oct 23 '12 at 15:13
  • It is a good idea to make equals and compareTo consistent, but a quick browse of the JDK source seems to indicate that this isn't the problem here. The sort code only appears to pay attention to compareTo and never calls equals. – Alex Oct 23 '12 at 15:21
  • @Alex: which source code are you referring to? Some implementations of set and maponly use compareTo but have contracts based on equals. Thus the inconsistency... – DaveFar Oct 23 '12 at 15:50
  • @just8laze: your equals() looks logically correct to me (besides exceptional cases, where you should check the class of `other`, and whether it is `null`). – DaveFar Oct 23 '12 at 15:54
  • @DaveBall http://hg.openjdk.java.net/jdk7/jdk7-gate/jdk/file/e947a98ea3c1/src/share/classes/java/util/ComparableTimSort.java line 715. This class is called from Arrays.sort(Object[]), and based on Google searches for the error message seems to be the likely source of the exception. – Alex Oct 23 '12 at 15:59