> On 9 Apr 2020, at 23:59, Gilles Sadowski <gillese...@gmail.com> wrote:
>
> Le jeu. 9 avr. 2020 à 23:20, Alex Herbert <alex.d.herb...@gmail.com
> <mailto:alex.d.herb...@gmail.com>> a écrit :
>>
>>
>>
>>> On 9 Apr 2020, at 21:36, Gilles Sadowski <gillese...@gmail.com> wrote:
>>>
>>> Le jeu. 9 avr. 2020 à 22:20, Alex Herbert <alex.d.herb...@gmail.com> a
>>> écrit :
>>>>
>>>>
>>>>
>>>>> On 9 Apr 2020, at 16:32, Gilles Sadowski <gillese...@gmail.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> Given this I am thinking that using ZERO when possible is a better
>>>>>> option and avoid 0 / -1.
>>>>>
>>>>> Hmm, then I'm both +0 and -0 (which is the same, right?)
>>>>> on this issue. ;-)
>>>>
>>>> Ironically the conversion to a double is a minor bug:
>>>>
>>>> Fraction.of(0, 1).doubleValue() == 0.0
>>>> Fraction.of(0, -01).doubleValue() == -0.0
>>>>
>>>> IEEE754 arithmetic for 0.0 / -1.0 creates a -0.0.
>>>>
>>>> Do we want to support -0.0?
>>>
>>> Why prevent it since it looks expected from the above call?
>>
>> Well, in the against argument -0.0 is an artefact of the IEEE floating point
>> format. It is not a real number.
>>
>> If we allow 0 / -1 as a fraction to mean something then we should really
>> support it fully which means carrying the sign of the denominator through
>> arithmetic as would be done for -0.0 (from the top of my head):
>>
>> -0.0 + -0.0 = -0.0
>> -0.0 + 0.0 = 0.0
>> 0.0 - -0.0 = 0.0
>> 0.0 - 0.0 = 0.0
>> 0.0 * 42 = 0.0
>> -0.0 * 42 = -0.0
>>
>> And so on...
>>
>> It is easier to exclude this representation from ever existing by changing
>> the factory constructor to not allow it.
>>
>> Note that Fraction.of(-0.0) creates 0 / 1. So the support for 0 / 1 is
>> inconsistent with conversion to and from double:
>>
>> Fraction.of(-0.0).doubleValue() == 0.0
>> Fraction.of(0, -1).doubleValue() == -0.0
>>
>> I have checked and Fraction.of(0, 1).compareTo(Fraction.of(0, -1)) is 0.
>> They evaluate to equal and have the same hash code. So this behaviour is
>> different from Double.compareTo, Double.equals and Double.hashCode which
>> distinguishes the two values.
>>
>> If fraction represented a signed number using the signed numerator and an
>> unsigned denominator, reduced to smallest form, then the representation of
>> zero is fixed. It would be 0 / 1 as you cannot have -0 as an integer.
>
> This seems to be the winning argument to transform all zero to
> canonical form.
I have updated Fraction and BigFraction to have a canonical form for Zero. Both
types pass the same tests for the constructor using a double and the
doubleValue() method does not return -0.0 for zero.
I tried to align the FractionTest and BigFractionTest to be more similar. This
should ease maintenance going forward.
I found that the use of a negative denominator had broken the compareTo
function. So I have fixed that.
The Fraction add/subtract methods were direct adaptions from the BigFraction
method. This was copied over from CM 3:
public Fraction add(final int i) {
return new Fraction(numerator + i * denominator, denominator);
}
This fails to detect overflow. So I have added tests for this and fixed the
implementation so it does not wrap the numerator (overflow).
All the fixes are small changes. One thing I noticed was that the constructor
using a double is limited to a maximum denominator of Integer.MAX_VALUE.
However with the support for Integer.MIN_VALUE as a denominator this
constructor does not allow for example:
@Test
public void test() {
Fraction f1 = Fraction.of(-1, Integer.MIN_VALUE);
Fraction f2 = Fraction.from(f1.doubleValue(), Integer.MIN_VALUE);
Assertions.assertEquals(f1, f2);
}
I will raise a ticket for this and investigate fixing it. The same limitation
is in BigFraction too.
Alex
