Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Equals And HashCode #26

Open
grantjforrester opened this issue Aug 12, 2016 · 0 comments
Open

Equals And HashCode #26

grantjforrester opened this issue Aug 12, 2016 · 0 comments

Comments

@grantjforrester
Copy link

grantjforrester commented Aug 12, 2016

Broadly I agree with the advice in this section - especially the first recommendation "Don't".

I have a couple of suggestions:

Collection Safety
I'd recommend some warning about the dangers of using mutable properties in your equals() and hashCode() methods. If you have a collection of such objects and the value of of a mutable property is changed, the collection will stop behaving as expected - it's effectively broken.

Hibernate Lazy-Loading Safety

Note: This information may only be pertinent to older versions of Hibernate - I don't know if it's still a thing as I haven't direct experience with newer versions of Hibernate.

Hibernate is a very commonly used persistence framework in the server-side enterprise and special care needs to be taken when defining equals() and hashCode() where entity relationships are defined as being lazily loaded.

In my experience when an entity is loaded lazily its class is actually a (CGLib?) proxy generated by Hibernate so getClass() == obj.getClass() does yield the expected result - as the class of the hydrated entity (say MyClass.class) is not the same as the proxy (say CGLib$$MyClass.class). The instanceof operator does yield the expected result as a the proxy is a subclass of the real class.

Furthermore, using direct field access in equals() and hashCode() methods does not yield expected results on lazily loaded class attributes. This is because synthetic accessor method are generated on the proxy, that when called, cause the property to be loaded and the field value set. Before the accessors are called direct field access returns null. So always use accessor function over direct field access.

In Summary

To be safe I'd recommend a modified version of your hand rolled method sticking with instanceof and only using accessors of immutable fields ...

@Override
public boolean equals(Object obj) {
    if (this == obj) 
      return true;
    if (obj == null)
      return false;
    if (!(obj instanceof MyClass)) // <- compare with instanceof 
      return false;
    MyClass other = (MyClass) obj;
    return Objects.equals(getImmutableField1(), other.getImmutableField1()) &&
        Objects.equals(getImmutableField2(), other.getImmutableField2());
  }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant