groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thibault Kruse <tibokr...@googlemail.com>
Subject Re: usage of NumberAwareComparator in extension methods and number oddities (a bit code review)
Date Thu, 03 Sep 2015 16:29:05 GMT
So the anti-commutativity is broken in my example because
NumberAwareComparator because DefaultTypeTransformation.compareTo does
not perform symmetric checks (only 'left' needs to implement
Comparable, not 'right').

Maybe this could already be fixed by
DefaultTypeTransformation.compareTo requiring both arguments to be of
equivalent type, such as both being Comparables. And possibly as an
alternative fallback both being of the same class and having a
compareTo Method (which would break less groovy code I guess).

This could further be relaxed to also allow comparing two instances
*of the same class* when they have a compareTo method. But comparing a
MyNumber with an Integer should IMO fail in
DefaultTypeTransformation.compareTo().

Another problem of anti-commutativity arises due to the weird
hashCode() comparison that never returns 0, even when both hashCode
values are the same.
Maybe it would be better to compare the System hashcodes
System.identityHashCode(x) rather than the hashcodes from the objects.
Maybe that way even a 0 value could be returned then.

Whether it makes sense at all to compare objects that way I don't know.

Comparing 0.0d and 0 as same without breaking equals-consistency seems
impossible, not sure what to do about that.

On Thu, Sep 3, 2015 at 5:17 PM, Jochen Theodorou <blackdrag@gmx.org> wrote:
> Am 03.09.2015 15:07, schrieb Thibault Kruse:
>>
>> That's a long read.
>
>
> and took hours to write
>
>> In a nutshell, it seems NumberComparator does
>> satisfy not the required anti-commutativity (
>> "The implementor must ensure that <tt>sgn(compare(x, y)) ==
>> -sgn(compare(y, x))</tt> for all <tt>x</tt> and <tt>y</tt>.")
when
>> used for Non-Comparables:
>>
>> x = new MyNumber(n: 1)
>> y = new Integer(0)
>> assert comp.compare(x, y) == - comp.compare(y, x)
>>         |    |       |  |  |  | |    |       |  |
>>         |    -1      |  0  |  1 |    -1      0  MyNumber(1)
>>         |            |     |
>> org.codehaus.groovy.runtime.NumberAwareComparator@3e77a1ed
>>         |            |     false
>>         |            MyNumber(1)
>>         org.codehaus.groovy.runtime.NumberAwareComparator@3e77a1ed
>>
>>
>> And it is not "consistent with equals", which means it should not be
>> used with sorted Sets or Maps ("should be used with caution").
>>
>> assert (comp.compare(new Float(0), new Integer(0)) == 0) ==  (new
>> Float(0).equals(new Integer(0)))
>>          |    |       |             |               |     |    |
>>      |      |
>>          |    0       0.0           0               true  |    0.0
>>      false  0
>>          |                                                false
>>          org.codehaus.groovy.runtime.NumberAwareComparator@fe18270
>>
>> Both violating anti-commutativity and not offering equals-consistency
>> make NumberAwareComparator an accident waiting to happen.
>
>
> the tip of the iceberg, yes
>
>> So +1 for making that less Groovy-relaxed and more Java-strict. But I
>> would say that about many things, without care for breaking existing
>> code (Because that's the only way to move Groovy into more production
>> code in the world).
>>
>> The Collection.minus() problem looks too complex to understand at a
>> glance.
>
>
>
> The question is, for both, what the change should be. If we want to support
> being able to do "assert 1.00g == 1.0g", as well as "assert 1==1.0d" and if
> we want to mimic that with NumberAwareComperator, then we have to violate
> "consistent with equals" and "anti-commutativity"
>
>
> bye blackdrag
>
>
> --
> Jochen "blackdrag" Theodorou
> blog: http://blackdragsview.blogspot.com/
>

Mime
View raw message