Closed GoogleCodeExporter closed 9 years ago
it might make sense to return Optional<T> instead
Original comment by SeanPFl...@googlemail.com
on 8 Aug 2013 at 3:19
[deleted comment]
FYI, checking that "MyClass.class != o.getClass()" rather than "!(o instanceof
MyClass)" is bad practice for equals, because it means that
myClass.equals(mySubclass) == false while mySubclass.equals(myClass) == true,
which breaks the equals() contract. For that matter,
mySubclass.equals(mySubclass) == false unless you override equals() in the
subclass, and again, that tends to break the equals contract.
In general, the best way to deal with ensuring the object passed to equals is
both non-null and an instance of the correct type is just:
if (o instanceof MyClass) {
MyClass other = (MyClass) o;
// check stuff
}
Original comment by cgdecker@google.com
on 8 Aug 2013 at 3:40
Thanks for your reply!
Checking for a `class` constant is indeed a bad idea because it causes touble
in subclasses. But isn't the `instanceof` approach suffering from the same
problem when `equals()` get's overridden?
Example:
static class A {
@Override
public boolean equals(Object obj) {
return obj instanceof A;
}
}
static class B extends A {
@Override
public boolean equals(Object obj) {
return obj instanceof B;
}
}
a.equals(b) => true
b.equals(a) => false
What about not checking for a `class` constant but for `this.getClass()`. This
would guarantee symmetry with subclasses.
new proposal:
<T> T sameTypeOrNull(T object, Object other)
Then `equals()` could be implemented like this:
public boolean equals(Object o) {
MyClass other = sameTypeOrNull(this, o);
return other != null
&& Objects.equal(this.fieldOne, other.fieldOne)
&& Objects.equal(this.fieldTwo, other.fieldTwo);
}
Original comment by j...@barop.de
on 15 Aug 2013 at 12:17
The `o instanceof MyClass` approach works with proxies, which is quite
important. Overriding it in a subclass can't be done in a consistent way.
Testing `o.getClass() == getClass()` is symmetrical and allows sane overriding,
but doesn't work with proxies. This is usually considered worse than the first
approach.
Using `o.getClass() == MyClass.class` seem to be plain wrong, neither proxies
nor overriding work.
There's no really good solution, as supporting both proxies and overriding
correctly requires a helper method, see
http://www.artima.com/lejava/articles/equality.html
Original comment by Maaarti...@gmail.com
on 15 Aug 2013 at 1:30
I see that the article I linked is a bit too long for what I wanted to clarify.
The point is that you sometimes want the subclass instances to be able to
`equals` to a superclass class instance (proxies or different implementation of
the same concept) and sometimes not (subclasses adding new properties like
ColoredPoint). For this a `canEqual` method is needed.
Original comment by Maaarti...@gmail.com
on 15 Aug 2013 at 2:35
> Testing `o.getClass() == getClass()` is symmetrical and allows sane
overriding, but doesn't work with proxies.
For this, I have a "magic" getNonProxyClass() and test
`getNonProxyClass(o.getClass()) == getNonProxyClass(getClass())`
when I need to support proxies (like in JPA entities) but do not want
`canEqual` stuff (which works unless someone forgets to override it).
Original comment by piotr.fi...@gmail.com
on 13 Nov 2013 at 12:49
This issue has been migrated to GitHub.
It can be found at https://github.com/google/guava/issues/<issue id>
Original comment by cgdecker@google.com
on 1 Nov 2014 at 4:12
Original comment by cgdecker@google.com
on 1 Nov 2014 at 4:17
Original comment by cgdecker@google.com
on 3 Nov 2014 at 9:08
Original issue reported on code.google.com by
j...@barop.de
on 7 Aug 2013 at 3:32