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 Mon, 07 Sep 2015 14:47:17 GMT
If you look at the commit I pointed to, it will be easier to discuss
the bugs. I list them here again. Class MyNumber is as you described
before, MyNumberComparable is the same implementing also Comparable
(see commit). I use numbers 49 and 50 because those are the int
representations of chars '1' and '2'.

MyNumberComparable numComp1 = new MyNumberComparable(49)

// works
checkCompareToSymmetricSmallerThan(numComp1, 50)
// Bug, NumberFormatException, comparison to all other Number types works
// checkCompareToSymmetricSmallerThan(numComp1, 50G)


// bug, returns 47?
// assert compareTo('aa1', '2' as Character) == 1

// bug, classCast exception
// assert compareTo('2' as Character, 'aa1') == 1

// bug, returns 47?
// assert compareTo("aa${1}", '2' as Character) == 1

// bug, classCast exception
//assert compareTo('2' as Character, "aa${1}") == 1

MyNumber number2 = new MyNumber(50)

// works
assert compareTo(49, number2) == -1
// bug: GroovyRuntimeException, but other way round works
//assert compareTo(number2, 49) == -1


Given there is no specification, the term "bug" might be stretching
it, but I believe from the rest of the code it can be inferred that in
those cases shown above the code does not behave as the programmer(s)
of the method intended for the method to behave. That's a bug for me.
In the absence of specifications all we can do is guess.




On Mon, Sep 7, 2015 at 2:53 PM, Jochen Theodorou <blackdrag@gmx.org> wrote:
>> So I would recommend fixing those, then adding some more tests with
>> other Comparables or Objects with compareTo, and make sure that it
>> behaves symmetrically and consistently.
>
>
> and what is the "expected" behaviour?

For DefaultTypeTransformation.compareTo I would say that it returns
values treating all numbers by their numeric value, being able to
compare all Strings/GStrings/Chars, and else comparing Comparables
only to Comparables, possibly considering instances having a compareTo
Method without implementing Comparable as duck-typed Comparables.

For NumberAwareComparator I would suggest the same behavior, throwing
Exceptions for any other case (e.g.
NumberAwareComparator.compare(new Object(), new Object()) should throw
an Exception, IMO).

Mime
View raw message