HashCodeBuilder is great, but ...

In real life, I'm a Java programmer, so I thought I'd bring a little technical content to this blog for a change. This issue came up today at work, when I spotted something weird in how some objects were behaving in a HashMap, something like this.

When doing domain modelling, one of the important things to get right is the definition of the equals() and hashcode() methods. I'm going to use a class that's used to represent a compound primary key in a Spring/JPA/Hibernate environment. The PaymentMethodId class has an owner, and two currency codes - one for the source of funds, and one for the destination of the funds, to allow for currency conversations between the two.

@Embeddable
public class PaymentMethodId extends CompositeId {

    @Column
    private Integer ownerNumber;

    @Column
    private String  sourceCurrencyCode;

    @Column
    private String  paymentCurrencyCode;

    /**
      * Compare unique business key (number, source code and payment code)
      */
    @Override
    public boolean equals(Object other) {
        if (this == other) {
           return true;
        }
        if (other == null || !(other instanceof PaymentMethodId)) {
           return false;
        }
        PaymentMethodId paymentMethodId = (PaymentMethodId) other;
        return ((this.ownerNumber == null ? paymentMethodId.ownerNumber == null
                                          : this.ownerNumber.equals(paymentMethodId.ownerNumber))
            && (this.paymentCurrencyCode == null ? paymentMethodId.paymentCurrencyCode == null
                                                         : this.paymentCurrencyCode.equals(paymentMethodId.paymentCurrencyCode))
            && (this.sourceCurrencyCode == null ? paymentMethodId.sourceCurrencyCode == null
                                                        : this.sourceCurrencyCode.equals(paymentMethodId.sourceCurrencyCode))
        );
    }

    @Override
    public int hashCode() {
        int result = 13;
        result = (ownerNumber != null ? 19 * result + ownerNumber : result);
        result = (paymentCurrencyCode != null ? 19 * result + paymentCurrencyCode.hashCode() : result);
        result = (sourceCurrencyCode != null ? 19 * result + sourceCurrencyCode.hashCode() : result);
        return result;
    }

}

I've left out constructors and getters and setters as these should be obvious.
As you can see, I like to make use of the 'trinary if' expression for brevity. Just imagine how much more complex these methods would be if written out in full!
Anyway, similar to Andy Beacock's post on this subject, we can firstly simplify the above as follows:

@Override
public boolean equals(Object obj) {
    if (obj instanceof PaymentMethodId) {
        PaymentMethodId other = (PaymentMethodId) obj;
        EqualsBuilder builder = new EqualsBuilder();
        builder.append(getOwnerNumber(), other.getOwnerNumber());
        builder.append(getSourceCurrencyCode(), other.getSourceCurrencyCode());
        builder.append(getPaymentCurrencyCode(), other.getPaymentCurrencyCode());
        return builder.isEquals();
    }
    return false;
}

@Override
public int hashCode() {
    HashCodeBuilder builder = new HashCodeBuilder();
    builder.append(getBrokerNumber());
    builder.append(getSourceCurrencyCode());
    builder.append(getPaymentCurrencyCode());
    return builder.toHashCode();
}

Note the use of getters instead of properties - as Andy mentioned.
However, the important bit that really is the reason for this post is the last line:

    return builder.toHashCode();

It is vital that you don't write:

    return builder.hashCode();

Notice the subtle difference? The hashCode() of the builder itself is worse than useful as it will vary every time the method is run, so any Hash-based collection code will find your object has a different hashCode, every time it asks, risking the integrity of the data.