FindBugs - how to solve EQ_COMPARETO_USE_OBJECT_EQUALS

Question

I am clueless here...

 1: private static class ForeignKeyConstraint implements Comparable<ForeignKeyConstraint> {
 2: String tableName;
 3: String fkFieldName;
 4: 
 5: public int compareTo(ForeignKeyConstraint o) {
 6:    if (this.tableName.compareTo(o.tableName) == 0) {
 7:            return this.fkFieldName.compareTo(o.fkFieldName);
 8:        }
 9:        return this.tableName.compareTo(o.tableName);
10:    }
11: }

In line 6 I get from FindBugs: Bug: net.blabla.SqlFixer$ForeignKeyConstraint defines compareTo(SqlFixer$ForeignKeyConstraint) and uses Object.equals()

Link to definition

I don't know how to correct this :S

Answer 1

This errors means that you're not overriding equals in ForeignKeyConstraint (and thus inheriting the equals from Object) so the following is not true (from the javadoc of compareTo):

It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y)). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."

To fix the FindBugs check, override equals - and hashCode - if it makes sense which is generally the case (or exclude the check for this class and document that your class violates this condition using the suggested note).

Answer 2

Have you tried overriding the equals method as well in SqlFixer.ForeignKeyConstraint?

I believe the basis of the warning is that, as stated in the definition, strange things can happen if you override compareTo and not equals.

For more information check out Joshua Bloch's Effective Java, 2nd Edition. Item 12 goes more in depth about the ins and outs of implementing Comparable and some of the things to look out for.

Answer 3

You can solve it by implementing an equals() method. Refer to the FindBugs definition:

"Generally, the value of compareTo should return zero if and only if equals returns true. If this is violated, weird and unpredictable failures will occur in classes such as PriorityQueue."

"It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y))."

Another example is the TreeSet. It implements equality checks by invoking compareTo, and a compareTo implementation that is inconsistent with equals makes the TreeSet violate the contract of the Set interface, which might lead to program malfunction.

Answer 4

It's telling you that there's the potential for compareTo() and equals() to disagree. And they should, really, never disagree.

The equals() method is being inherited from java.lang.Object, which by default checks to see if two objects are the same instance. Your compareTo method is comparing objects are based on tableName and fkFieldName. So you'll potentially find yourself in a situation where compareTo states that two objects are the same (because tableName and fkFieldName match), but equals states they are different (because they're different instances).

There are a few java APIs that depend on compareTo and equals being consistant; this is part of the java language and is considered a core language contract. Ideally implement an equals (and hashcode) method to check for equality based on tableName and fkFieldName.