-
-
Save rocketraman/653af25ee3bf72ca497f to your computer and use it in GitHub Desktop.
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(); | |
} | |
} |
@trejkaz
It's still using
Objects
. Alsoinstanceof
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.
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.
I can make a subclass such that
b.equals(a)
returns true but youra.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.
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.
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 ofAddress
, 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.
It's still using
Objects
. Alsoinstanceof
would be better practice, would it not?