Skip to content

Instantly share code, notes, and snippets.

@rocketraman
Forked from rocketraman/equals-hash-tostring.java
Last active April 29, 2020 01:48
Show Gist options
  • Save rocketraman/653af25ee3bf72ca497f to your computer and use it in GitHub Desktop.
Save rocketraman/653af25ee3bf72ca497f to your computer and use it in GitHub Desktop.
Example of JDK7+ Objects equals, hash and Guava 18+ toString implementations
import com.google.common.base.MoreObjects;
import java.util.Objects;
public class Address {
...
@Override
public boolean equals(Object obj) {
if (obj == null) return false;
if (getClass() != obj.getClass()) return false;
final Address other = (Address) obj;
return Objects.equals(this.houseNumber, other.houseNumber)
&& Objects.equals(this.street, other.street)
&& Objects.equals(this.city, other.city)
&& Objects.equals(this.stateOrProvince, other.stateOrProvince)
&& Objects.equals(this.country, other.country);
}
@Override
public int hashCode() {
return Objects.hash(
this.houseNumber, this.street, this.city, this.stateOrProvince, this.country);
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("houseNumber", houseNumber)
.add("street", street)
.toString();
}
}
@hakanai
Copy link

hakanai commented Apr 26, 2020

It's still using Objects. Also instanceof would be better practice, would it not?

@rocketraman
Copy link
Author

rocketraman commented Apr 28, 2020

@trejkaz

It's still using Objects. Also instanceof would be better practice, would it not?

Yes, but now its the Objects built into the JDK (import java.util.Objects;), not Guava.

Regarding instanceof it is valid in some cases but the class comparison is safer as it always preserves the symmetry aspect of the equals contract, and therefore more appropriate for a generic implementation such as this. See this discussion for more details: https://stackoverflow.com/q/596462/430128.

@hakanai
Copy link

hakanai commented Apr 28, 2020

I can make a subclass such that b.equals(a) returns true but your a.equals(b) returns false, which isn't really what I'd call maintaining symmetry. To make that claim, I believe you have to make your class final. At which point, why not use instanceof anyway? You also get to avoid the null check.

@rocketraman
Copy link
Author

I can make a subclass such that b.equals(a) returns true but your a.equals(b) returns false

@trejkaz I don't see how. If either a or b is a different class, whether subclass or not, would mean a.getClass() != b.getClass(), thus maintaining symmetry. If you can violate symmetry with this code, please post an example.

@hakanai
Copy link

hakanai commented Apr 29, 2020

public class EvilAddress extends Address {
    @Override
    public boolean equals(Object other) {
        return true;
    }
}

Address a = new Address(...);
Address b = new EvilAddress(...);

a.equals(b);  // false
b.equals(a);  // true

Granted it's an evil example, but it illustrates the point. If you make Address final it solves the issue completely.

On the other hand, I think that this.getClass() == that.getClass() is also too rigid - it prevents me substituting a legitimate alternative implementation of Address, which happens to want to consider itself equal to the normal one.

@rocketraman
Copy link
Author

That's a trivially silly example. Making Address final, nor using instanceof, does not "solve" this:

public class Foo {
  public boolean equals(Object other) {
    return true;
  }
}

Address a = new Address(...);
Foo f = new Foo();

a.equals(f);  // false
f.equals(a);  // true

On the other hand, I think that this.getClass() == that.getClass() is also too rigid - it prevents me substituting a legitimate alternative implementation of Address, which happens to want to consider itself equal to the normal one.

Yes, that's the Liskov Substitution Principle, and yes this implementation does not allow that. This gist isn't meant as a be-all and end-all that works in every situation, but a general implementation that works in many cases. Coming back full circle, as I said earlier, there are times when instanceof is the right approach, if subclass substitution is important. However, as Angelika Lankers says:

Whether or not this is a problem fully depends on the semantics of the class and the purpose of the extension.

BTW, I also agree that "final by default" is a good idea -- if a class is meant to be extended it should be explicitly written as such. Classes are final by default in Kotlin, unless explicitly marked as "open" by the author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment