Using getters in equals() method

I have an uncontrollable urge to refactor. To a fault. Most everything. If I was cleaning a campsite, I would be putting in indoor plumbing. So, when I opened an old class I changed the old hashcode and equals methods with our team’s standard use of HashBuilder and EqualsBuilder. (The reason for opening the class was to add a toString method.)

@Override
public boolean equals(Object obj) {
    if (this == obj) {
        return true;
    }
    if (obj == null) {
        return false;
    }
    if (!(obj instanceof ${enclosing_type})) {
        return false;
    }
    ${enclosing_type} other = (${enclosing_type}) obj;
    ${builderType:newType(org.apache.commons.lang3.builder.EqualsBuilder)} ${builder:newName(builderType)} = new ${builderType}();
    ${builder}.append(this.${field}, other.${field}());${cursor}

    return ${builder}.isEquals();
}

When I ran the application again (I was trying to find a bug) I wasn’t getting to the same place in the code as before. Stepping through with the debugger I noticed that I was no longer returning “true” during a comparison of the object I had just changed. So I went back to the old version of the equals method. I could not visually see a difference. So I wrote a JUnit test case to test the equals method, specifically for symmetry. That didn’t get me very far; everything passed as expected.

So I ran the program through the debugger stopping in the equals method for comparison. It looked like the passed in object had nulls for everything. I changed my JUnit tests to test for the same setup, but it worked as expected.

So I went back and created 2 equals methods. Then I had each run and if there was a difference between the old equals method and the new equals method, I would just exit the program:

@Override
public boolean equals(final Object obj) {
    // TEMPORARY!!! 
    final boolean newEquals = newEquals(obj);
    final boolean oldEquals = oldEquals(obj);
    if (newEquals != oldEquals) {
        newEquals(obj); // for debugging, I can step through the method again 
        oldEquals(obj); // for debugging, I can step through the method again
        LOGGER.error("newEquals (" + newEquals + ") doesn't equal oldEquals (" + oldEquals + ")!!!");
        LOGGER.error("this = " + this);
        LOGGER.error("obj = " + obj);
        System.exit(1); // TEMPORARY!!!
    } 
    return oldEquals;
}

I re-ran the JUnit test, but still no luck.

I re-ran the application and viola the equals methods were different and the app exited. But what was going on?

I added a breakpoint in the equals method and looked at the state of the variables again. That’s when I noticed the my object being passed in wasn’t what I expected, but some kind of proxy object. I expanded the object and found a target variable. I expanded that and saw the values matched the “this” object!

Equals Comparison Screenshot

I looked back at the equals methods. The original equals method has get() and mine had just the direct access to the member variable. I step throw the call to the get method and found the proxy will load the member variables to their real values.

So, the lesson I learned was that I need to be calling the get methods in my equals method for the object being passed in. I changed my equals template to add a long-winded explanation for the change. Here’s my Eclipse template:

@Override
public boolean equals(Object obj) {
    if (this == obj) {
        return true;
    }
    if (obj == null) {
        return false;
    }
    if (!(obj instanceof ${enclosing_type})) {
        return false;
    }
    ${enclosing_type} other = (${enclosing_type}) obj;
    ${builderType:newType(org.apache.commons.lang3.builder.EqualsBuilder)} ${builder:newName(builderType)} = new ${builderType}();
    // You want to use get() methods because ${enclosing_type} might by a "proxy" that libraries (such as Hibernate with CGLIB) will create. (Hibernate uses proxies for lazy loading.) And if the object is not loaded, then access to the fields directly will yield nulls. But calls to the get() methods are intercepted and the proxy returns the correct value. See https://forum.hibernate.org/viewtopic.php?t=946468
    ${builder}.append(this.${name}, other.get${name}());${cursor}

    return ${builder}.isEquals();
}

Thanks go to this post for helping me understand the issue.