Note: I merely modified the equivalent rule for equals to make this rule.
It also seems like I'm sort of just copying from the Java standard,
but I can't think of any reason why if we have an equals rule
we should not have a compareTo rule, since it is so often used with equals.
This rule could be extended to deal with Comparator as well; since they go together.
The general usage contract for compareTo() has been put forth verbatim from the Java specification:
The implementor must ensure sgn(x.compareTo
) == -sgn(y.compareTo
) for all x and y. (This implies that x.compareTo must throw an exception iff y.compareTo throws an exception.)
The implementor must also ensure that the relation is transitive: (x.compareTo>0 && y.compareTo(z)>0) implies x.compareTo(z)>0.
Finally, the implementor must ensure that x.compareTo==0 implies that sgn(x.compareTo(z)) == sgn(y.compareTo(z)), for all z.
Do not violate any of five conditions while overriding the compareTo method.
Noncompliant Code Example
This noncompliant code example violates the third condition in the contract.
Consider a Card that considers itself equal to any card of the same suit; otherwise it orders based on rank.
public final class Card implements Comparable{
private String suit;
private int rank;
public Card(String s, int r) {
if (s == null)
throw new NullPointerException();
suit = s;
rank = r;
}
public boolean equals(Object o) {
if (o instanceof Card){
Card c=(Card)o;
return suit.equals(c.suit) || (rank == c.rank);
}
return false;
}
//this method violates its contract
public int compareTo(Object o){
if (o instanceof Card){
Card c=(Card)o;
if(suit.equals(c.suit)) return 0;
return c.rank - rank;
}
throw new ClassCastException();
}
public static void main(String[] args) {
Card a = new Card("Clubs", 2);
Card b = new Card("Clubs", 10);
Card c = new Card("Hearts", 7);
System.out.println(a.compareTo(b)); //returns 0
System.out.println(a.compareTo(c)); //returns a negative number
System.out.println(b.compareTo(c)); //returns a positive number
}
}
Compliant Solution
Do not try to inter-operate with String from the equals method. The new equals method is highlighted in this compliant solution.
public final class CaseInsensitiveString {
private String s;
public CaseInsensitiveString(String s) {
if (s == null)
throw new NullPointerException();
this.s = s;
}
public boolean equals(Object o) {
return o instanceof CaseInsensitiveString &&
((CaseInsensitiveString)o).s.equalsIgnoreCase(s);
}
public static void main(String[] args) {
CaseInsensitiveString cis = new CaseInsensitiveString("Java");
String s = "java";
System.out.println(cis.equals(s)); //returns false now
System.out.println(s.equals(cis)); //returns false now
}
}
Noncompliant Code Example
This noncompliant example violates transitivity though it follows the symmetry condition. This is because the first two statements print true while the third prints false. A practical implementation issue is intermingling of java.sql.Timestamp and java.util.Date classes. There is a disclaimer about the erratic behavior in the documentation for the Timestamp class.
public class Card {
private final int number;
public Card(int number) {
this.number = number;
}
public boolean equals(Object o) {
if (!(o instanceof Card))
return false;
Card c = (Card)o;
return c.number == number;
}
}
class XCard extends Card {
private String type;
public XCard(int number, String type) {
super(number);
this.type = type;
}
public boolean equals(Object o) {
if (!(o instanceof Card))
return false;
//normal Card, do not compare type
if (!(o instanceof XCard))
return o.equals(this);
//It is an XCard, compare type as well
XCard xc = (XCard)o;
return super.equals(o) && xc.type == type;
}
public static void main(String[] args) {
XCard p1 = new XCard(1, "type1");
Card p2 = new Card(1);
XCard p3 = new XCard(1, "type2");
System.out.println(p1.equals(p2)); //returns true
System.out.println(p2.equals(p3)); //returns true
System.out.println(p1.equals(p3)); //returns false, violating transitivity
}
}
Compliant Solution
"There is simply no way to extend an instantiable class and add an aspect while preserving the equals contract." This implies that composition must be preferred over inheritance in this case. This is done by giving the XCard class a private card field and providing a a public viewCard method. [[Bloch 08]]
public class Card {
private final int number;
public Card(int number) {
this.number = number;
}
public boolean equals(Object o) {
if (!(o instanceof Card))
return false;
Card c = (Card)o;
return c.number == number;
}
}
class XCard extends Card {
private String type;
private Card card;
public XCard(int number, String type) {
super(number);
this.type = type;
}
public Card viewCard() {
return card;
}
public boolean equals(Object o) {
if (!(o instanceof XCard))
return false;
XCard cp = (XCard)o;
return cp.card.equals(card) && cp.type.equals(type);
}
public static void main(String[] args) {
XCard p1 = new XCard(1, "type1");
Card p2 = new Card(1);
XCard p3 = new XCard(1, "type2");
System.out.println(p1.equals(p2)); //returns false
System.out.println(p2.equals(p3)); //returns false
System.out.println(p1.equals(p3)); //returns false
}
}
TODO: Add condition for hashcode
Risk Assessment
Violating the general contract when overriding the equals() method can lead to unexpected results.
Rule |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
|---|---|---|---|---|---|
MET30-J |
low |
unlikely |
medium |
P2 |
L3 |
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
[[API 06]] method equals()
[[Bloch 08]] Item 8: Obey the general contract when overriding equals
[[Darwin 04]] 9.2 Overriding the equals method
MET03-J. For methods that return an array or collection prefer returning an empty array or collection over a null value 09. Methods (MET) MET31-J. Ensure that hashCode() is overridden when equals() is overridden