Generate final equals/hashCode methods
Jan Ouwens
Finally decided to start using Lombok as well. It'll save me a lot of
typing in EqualsVerifier's unit tests!
One thing I'm missing, though, is a way for @EqualsAndHashCode to
generate final equals and hashCode methods. (So that's the methods
themselves, not the class as a whole.) I discussed this with Roel last
week, and we agreed I'd bring it up here.
Sometimes, it can be useful to leave a class open for subclassing,
without allowing the user to change the behaviour of equals and
hashCode. It's easy to mess up equals, especially when inheritance is
involved, so sometimes you want to prevent that by marking it final,
while still making it possible to override other methods in the class.
Maybe it's possible to add a flag to @EqualsAndHashCode that can do
this?
(Obviously, it should always mark both method as final, not just one
of them.)
Regards,
Jan
Reinier Zwitserloot
that would NOT break the equals/hashCode contract is one that does not
add any new state at all. Is that particularly common?
I also have to question the wisdom in trusting folks with the power to
subclass your project while not trusting them with getting equals and
hashCode right. Programming is turing-complete, which means shooting
yourself in the foot is always an option. You can't prevent folks from
doing it. You can only make nice APIs and document them appropriately.
Jan Ouwens
> I'm not sure that's a good idea. The only sensible subclass possible
> that would NOT break the equals/hashCode contract is one that does not
> add any new state at all. Is that particularly common?
AbstractList could have had final equals and hashCode methods if it
weren't for one or two uncommon, special purpose subclasses. Though
I'll admit you probably wouldn't use Lombok if you were designing an
ADT. You might also need it if you use the Template Method pattern in
a class that also needs equals/hashCode.
So no, it's probably not common. But it can happen.
> I also have to question the wisdom in trusting folks with the power to
> subclass your project while not trusting them with getting equals and
> hashCode right. Programming is turing-complete, which means shooting
> yourself in the foot is always an option. You can't prevent folks from
> doing it. You can only make nice APIs and document them appropriately.
having a few safeties on so I don't do it accidentally. And making an
entire class final seems a bit harsh when you only want to protect
equals and hashCode. Also, in a sense, final is part of the API, and
as a bonus, the compiler will enforce it. It's a lot better than a
"please do not override!" comment in the documentation :).
But I object to this kind of reasoning. It's an interesting discussion
for sure, but what it boils down to, is: do you want to say "This is
bad programming style, and I do not want people who use my product to
program this way" ? So allow me to turn this line of reasoning around
on you.
Some people don't use final at all. Some use it everywhere they can,
even on local variables in small scopes. Some people like to make
their classes final, some don't. All these ways of programming are
compatible with Lombok, except for the one where people like to use
final on their methods (like me). If I want to make my equals method
final, I'll have to write all the boilerplate that Lombok is supposed
to get rid of for me, and it's not trivial either, like a getter would
be.
So, not supporting final equals/hashCode seems like an arbitrary
limitation to me. The only difference with the other "styles of
final" (fields, classes), is that you'll have to write code to support
it. So you have to decide: are you, indeed, fundamentally against
this? Do you think it's time better spent elsewhere? Or are you trying
to mask laziness with an interesting though irrelevant discussion on
the wisdom of programming in Turing-complete languages :)?
Regards,
Jan
Reinier Zwitserloot
There is a somewhat legitimate alternate interpretation of the equals/
hashCode contract, which is used by amongst other things AbstractList,
at least more or less. In this version, a certain type serves as
'equivalence reference point', and it is from this perspective that
equivalence is determined for _all_ instances of this class or any
subclass of it. In other words, any two lists are equal if they
contain the same elements in the same order, regardless of what kind
of type either list is.
Unfortunately there's no halfway in this; it has to be the ONLY way,
even for comparisons between 2 instances of the same subtype. Thus, if
one creates a coloured list, which adds a color property to a list,
then a red list containing "a", "b", and "c" has to be entirely equal
to a blue list containing "a", "b", and "c", and even produce the same
hashCode, or it won't work. This is severely troublesome, as it is not
possible to convey the notion that subclasses cannot add state like
this, except in documentation. Nevertheless, there are legitimate
reasons to do this. java.util.List is exhibit A for this model.
The most pragmatic implementation of such an 'equivalence level'
notion Roel and I came up with involves 2 changes to how equals/
hashCode is currently generated by lombok:
A. instead of using other.getClass() == this.getClass(), use other
instanceof MyType.
B. make equals and hashCode final.
[C. optional: add javadoc explaining the vagaries of what it means to
work with equivalence reference point style equality.]
Both of course from the perspective of the class that declares itself
as the equivalence reference point, i.e. AbstractList.
This isn't about programming style; this is about the rules of the
hashCode/equals contract. The way lombok interprets it, ArrayList and
LinkedList are _WRONG_ - that is, they use a broken equals/hashCode
implementation, as an arraylist can be equal to a linkedlist (not
something lombok's understanding of equals/hashCode ascribes to).
In case you are wondering, its most assuredly not laziness; adding
such a parameter is about 2 minutes of work. We tend to respond with a
no anytime any new feature requires tacking an obscure-ish parameter
on top of any of the lombok annotations. Our basic reasoning when
doing this goes like so:
"I can think of 50 other obscure little changes one might wish for in
rare and very specific circumstances to the way lombok generates
things. Why are those not also deserving of an option?"
If we can't answer that question, the feature won't go in, because as
good as it might sound, having 50 different parameters on @Data or
@EqualsAndHashCode is clearly inferior to the lombok of today.
Having said all that, supporting the equivalence reference point
notion is something that really does seem worthy of an option. Though,
far more likely we'd use an alternate annotation, which is mutually
exclusive with @EqualsAndHashCode. In retrospect, @EqAHC was a bad
name. @EquivalenceRelation or @FieldBasedEquivalence or some such
would have been far smarter. As that kind of equivalence relation is
relatively rare and comes with far more caveats, its not high on the
priority list; Lombok's main goal is for common and verbose
boilerplate. list-style equivalence is verbose, but not particularly
common, so not really within lombok's domain.
Maaartin-1
> Unfortunately there's no halfway in this; it has to be the ONLY way,
> even for comparisons between 2 instances of the same subtype. Thus, if
> one creates a coloured list, which adds a color property to a list,
> then a red list containing "a", "b", and "c" has to be entirely equal
> to a blue list containing "a", "b", and "c", and even produce the same
> hashCode, or it won't work. This is severely troublesome, as it is not
> possible to convey the notion that subclasses cannot add state like
> this, except in documentation. Nevertheless, there are legitimate
> reasons to do this. java.util.List is exhibit A for this model.
I can't imagine any class implementing List which really needs
additional state, but I know, there are too many things I can't imagine.
However, I'm quite sure, there's a way to implement a correct equals
even for lists with colored subclasses, so that a red is unequal to the
blue one, should I try to describe it?
> The most pragmatic implementation of such an 'equivalence level'
> notion Roel and I came up with involves 2 changes to how equals/
> hashCode is currently generated by lombok:
>
> A. instead of using other.getClass() == this.getClass(), use other
> instanceof MyType.
As already discussed here, this is the way to go when dynamic proxies
get used.
> B. make equals and hashCode final.
This should be independent of A, shouldn't it? Actually, it's fine even
for dynamic proxies (unless they think they must override it), and I
don't see any case where just one of A and B is needed.
> [C. optional: add javadoc explaining the vagaries of what it means to
> work with equivalence reference point style equality.]
Note, that it's not possible to automatically generate EqAHC for List or
any similar class coming to my mind.
> "I can think of 50 other obscure little changes one might wish for in
> rare and very specific circumstances to the way lombok generates
> things. Why are those not also deserving of an option?"
>
> If we can't answer that question, the feature won't go in, because as
> good as it might sound, having 50 different parameters on @Data or
> @EqualsAndHashCode is clearly inferior to the lombok of today.
Sure, and setting all the options could in fact increase boilerplate
since it could get longer than the generated code. :D
> Having said all that, supporting the equivalence reference point
> notion is something that really does seem worthy of an option. Though,
> far more likely we'd use an alternate annotation, which is mutually
> exclusive with @EqualsAndHashCode. In retrospect, @EqAHC was a bad
> name. @EquivalenceRelation or @FieldBasedEquivalence or some such
> would have been far smarter.
I disagree.
Aren't mutually exclusive annotations the more complicated thing? Aren't
they going to use about the same parameters?
The annotated class is no EquivalenceRelation at all (you just define
one on its instances), so it shouldn't be annotated this way.
With using getters by default, you just can't call it
FieldBasedEquivalence. Something like PropertyBasedEquivalence would be
better, but it gets too long. This and the fact that the annotated class
is no equivalence makes me happy with EaHC again.
I'd also use preferFields or ignoreGetters instead of doNotUseGetters,
but this doesn't matter and comes too late, anyway.
Reinier Zwitserloot
contract. The contract says that this must be true:
if (a.equals(b) && b.equals(c)) assert a.equals(c);
Take for 'a' a red empty list, for 'c' a blue empty list, and then
transpose the first equals, which is legal, because the contract also
says that this must be true:
if (a.equals(b)) assert b.equals(a);
now we have:
if (b.equals(a) && b.equals(c)) assert a.equals(c);
b, being a plain empty list, will report true for both a blue empty
list and a red empty list. Therefore a blue empty list and a red empty
list must be equals() to each other, or you break the contract.
Using instanceof for dynamic proxies introduces a hard-to-dodge
opportunity for folks to break the equals/hashCode contract by
subclassing and adding state of any sort, and/or overriding equals/
hashCode. Therefore, using instanceof instead of .getClass() must go
hand in hand with making hashCode and equals final, *and* we need to
document that state relevant for equality can no longer be added.
Yes, FieldBasedEquivalance isn't really the right name either, though,
while we use getters, all lombok annotations are currently very much
field based. The presence of a getter doesn't mean we'll call it. We
look for a field, and use it, though we'll access it via getX()
instead of x if you also have a getter for it.
If we ever do tackle this notion (as I mentioned, due to its rarity
its low on the list), we'll think it through properly :P
Jan Ouwens
Both options (a switch or a separate, mutually exclusive annotation)
are fine by me. A pity that it's not high on your list of priorities,
but I understand :).
On Aug 17, 4:32 pm, Reinier Zwitserloot <reini...@gmail.com> wrote:
> If rather doubt you can make coloured lists work without breaking the
> contract.
because the implementation in AbstractList would have to be changed.
If you're interested, I know of two ways:
* The "blindlyEquals" approach
Dr Dobbs, 2002: http://www.drdobbs.com/java/184405053
Be sure to also read the followup on http://tal.forum2.org/equals as
it contains some corrections.
* The "canEqual" approach
Artima weblogs, 2009: http://www.artima.com/lejava/articles/equality.html
This is basically chapter 28.2 of the "Staircase" Scala book, adapted
for Java.
I prefer the "canEqual" approach. It seems cleaner to me.
Regards,
Jan
Maaartin-1
> If rather doubt you can make coloured lists work without breaking the
> contract. The contract says that this must be true:
>
> if (a.equals(b) && b.equals(c)) assert a.equals(c);
>
> Take for 'a' a red empty list, for 'c' a blue empty list, and then
> transpose the first equals, which is legal, because the contract also
> says that this must be true:
>
> if (a.equals(b)) assert b.equals(a);
>
> now we have:
>
> if (b.equals(a) && b.equals(c)) assert a.equals(c);
>
> b, being a plain empty list, will report true for both a blue empty
> list and a red empty list.
It will not. You're assuming that the actual class and all additional
attributes get ignored in the base class equals, but this is not the way
to go.
> Therefore a blue empty list and a red empty
> list must be equals() to each other, or you break the contract.
Using your assumption, yes. In case I'm right, no.
> Using instanceof for dynamic proxies introduces a hard-to-dodge
> opportunity for folks to break the equals/hashCode contract by
> subclassing and adding state of any sort, and/or overriding equals/
> hashCode. Therefore, using instanceof instead of .getClass() must go
> hand in hand with making hashCode and equals final,
Not necessarily. Let me cite you:
Programming is turing-complete, which means shooting yourself in the
foot is always an option. You can't prevent folks from doing it. You can
only make nice APIs and document them appropriately.
Making EaHC final may be fine for dynamic proxies and may prevent some
errors. Making equals work *with* additional state in subclasses
requires overriding.
Have a look at my example at
http://dl.dropbox.com/u/4971686/lombok/EqualsDemo1.java
There's still a problem, I've got a solution, but I'm looking for a
simpler one.
> *and* we need to
> document that state relevant for equality can no longer be added.
> Yes, FieldBasedEquivalance isn't really the right name either, though,
> while we use getters, all lombok annotations are currently very much
> field based. The presence of a getter doesn't mean we'll call it. We
> look for a field, and use it, though we'll access it via getX()
> instead of x if you also have a getter for it.
That's new to me and not really obvious from the documentation. I could
find it out when reading the javadoc for EqualsAndHashCode.of and
thinking hard....
> If we ever do tackle this notion (as I mentioned, due to its rarity
> its low on the list), we'll think it through properly :P
IMHO, an equals compatible with dynamic proxies is a very commonly
needed thing. Agreed, anything beyond that is rare.