groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jochen Theodorou <blackd...@gmx.org>
Subject Re: groovy git commit: GROOVY-7876: ClassCastException when calling DefaultTypeTransformation#compareEqual (closes #368)
Date Mon, 25 Jul 2016 07:05:11 GMT
On 24.07.2016 14:56, Paul King wrote:
> I think the GString check can be moved up as you suggest to improve
> the readability of the code without breaking existing behavior, so
> I've done that.
>
> Whether to go with your other suggestions is less clear to me.
>
> If we assume the check in EqualsTest#testParentChildrenEquals remains
> valid, i.e. we want symmetry of equals for java.util.Date and
> java.sql.Time which compareTo gives us so long as we have both the
> left<<right and right<<left isAssignable checks, then the current code
> in master seems reasonable. And once we have the right<<left case, we
> need the Object check as well for GROOVY-4046.

If we do this only for java.util.Date and family, then we should 
probably special case it much like we do for GString

[...]
> I guess the underlying assumption in the existing code is that for
> Comparable classes, the compareTo method is more likely to provide a
> "business/human" friendly equality check than equals (since the
> contracts around equals behavior have more specific strict
> requirements), as illustrated by e.g. BigDecimal. The question is
> then, do we want to continue honoring this assumption or provide that
> only for the specific number widening that we do anyway.
>
> So, if we want to change the code to something like:
>
> if (equalityCheckOnly) {
>      return left.equals(right) ? 0 : -1;
> } else {
>      return ((Comparable) left).compareTo(right);
> }
>
> then something like the following could no longer be made to work:
>
> class NumberHolder implements Comparable<NumberHolder> {
>    Number n
>    int compareTo(NumberHolder other) {
>      n <=> other.n
>    }
> }
>
> assert new NumberHolder(n: 10L) == new NumberHolder(n: 10.0G)
>
> For me, such a breaking change sounds more like a 3.0 thing.

My suggestions means more or less to make == go to equals and <=> to 
compareTo, of course more according to our special rules and including 
some widening and stuff.. But this implementation would be more near 
Java in my eyes, while still being comfortable enough.

For 2.0 I would see two more ways:
(a) define our own method for to do any of <,<=,==,<=>,>=,>. With the 
problem of having to have a fallback, that depends on equals and 
compareTo and most likely will have the same problem, plus the problem 
of not being so Java friendly.
(b) have an equals == and a compareTo ==, with differing syntax... maybe 
time to resurrect === and have <, <=, ===, <=>, >=, > ... well, or any 
other operator we can come up with...

bye Jochen

Mime
View raw message