Mar 06 2012
By dracox debug

Subtle equals bug in Java

The findbugs fairy discovered a little issue in my projects.

Bug: com.bfm.appl.blimp.marketdata.FXPricingSecurity overrides equals in PricingSecurity and may not be symmetric
Patternid: EQ_OVERRIDING_EQUALS_NOT_SYMMETRIC, type: Eq, category: CORRECTNESS

This class defines an equals method that overrides an equals method in a superclass. Both equals methods methods use instanceof in the determination of whether two objects are equal. This is fraught with peril, since it is important that the equals method is symmetrical (in other words, a.equals(b) == b.equals(a)). If B is a subtype of A, and A’s equals method checks that the argument is an instanceof A, and B’s equals method checks that the argument is an instanceof B, it is quite likely that the equivalence relation defined by these methods is not symmetric.

Clear as mud? Let’s have a look at an example:

class StarFighter {
    string name;
    //bunch of other star fighter related stuff here
    ...
    @Override
    public boolean equals(Object obj) {
        if (obj == null) return false;
        if (!(obj instanceof StarFighter)) return false;

        return this.name.equals(((StarFighter)obj).name);
    }
}

So far so good, we have our beautiful StarFighter class ready to roll, and the equals method works, more or less.

However what if we what to specialise it a bit, say get our plan for a X-wing StarFighter going.

class XWing extends StarFighter {
    //bunch of cool XWing stuff
    ...
    @Override
    public boolean equals(Object obj) {
        if (obj == null) return false;
        if (!(obj instanceof XWing)) return false;

        return this.name.equals(((XWing)obj).name);
    }
}

Oh no! Here’s where we violate the symmetrical property of equals function.


StarFighter s1 = new StarFighter();
StarFighter x1 = new XWing();
s1.setName("Steve");
x1.setName("Steve");
assertFalse(x1.equals(s1)); // yep, x1 is non-equivalent to a star fighter.
assertFalse(s1.equals(x1)); // fail...

This is pretty unpredictable behaviours right here. Depends on which StarFighter you use as the base for compare, you get complete opposite results.

How did I fixed this? In our case it made sense to make StarFighter into an abstract class and removed the equals method, and let the YWing class to implement it’s own equals. Which actually made more sense, so as it turns out, the bug was an indication of poor architecture.

Tagged ,