Mailing-List: contact dev-help@groovy.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@groovy.apache.org
MIME-Version: 1.0
Reply-To: paulk@asert.com.au
In-Reply-To: <5795BA27.9000808@gmx.org>
References: <2e32e62080694924bfbc444e266f5421@git.apache.org>
 <5791F59B.9070101@gmx.org> <CAMbkE7QOsZjcUUQ8JF08itKOWAgfkd1TzTR-sk7cLW7boFx0ZQ@mail.gmail.com>
 <57921D09.4020201@gmx.org> <CAMbkE7R=iG3e65phbWDmoE39R9KpqPqehqd1kiortg4mF2Wfuw@mail.gmail.com>
 <5795BA27.9000808@gmx.org>
From: Paul King <paulk@asert.com.au>
Date: Mon, 25 Jul 2016 18:48:41 +1000
Message-ID: <CAMbkE7SoZ_2TsBzqiPsJaU5EE4aHXj-6QUZhVux5UdLRHq=q=A@mail.gmail.com>
Subject: Re: groovy git commit: GROOVY-7876: ClassCastException when calling
 DefaultTypeTransformation#compareEqual (closes #368)
To: dev@groovy.apache.org
Content-Type: text/plain; charset=UTF-8
archived-at: Mon, 25 Jul 2016 08:48:55 -0000

Yes, I generally agree with what you are suggesting. We do seem to
talk about this a lot from time to time but struggle to make much real
progress. I guess we need to start filling in the parts of this puzzle
with poorly defined behavior with some additional tests. I'll try to
give some more attention to some of the outstanding PRs around custom
numbers if I get time.

Cheers, Paul.

On Mon, Jul 25, 2016 at 5:05 PM, Jochen Theodorou <blackdrag@gmx.org> wrote:
> 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