On 09/04/2020 14:38, Gilles Sadowski wrote:
Le jeu. 9 avr. 2020 à 14:09, Alex Herbert <alex.d.herb...@gmail.com> a écrit :

On 09/04/2020 12:04, Alex Herbert wrote:
So if we are to support 0/-1 then I will add this to the standard test
cases to make sure it is correctly implemented.
So I did this. No major functionality failures.

However there is a discrepancy between multiply and other functions.

Multiply will return the form 0 / 1 for zero when either argument is zero.


Divide will return the current instance if the instance is zero.

Add/subtract will return the current instance if the instance to add is
zero.

Pow will return the current instance if the instance is zero.


So you get this:

0 / -1   multiply   1 / 3    == 0 / 1


0 / -1   divide   1 / 3    == 0 / -1


0 / -1   add   0 / 1    == 0 / -1

0 / -1   pow   1    == 0 / -1

0 / -1   pow   2    == 0 / -1


In the later case you may actually expect pow(0 / -1, 2) to return 0 /
1. The result would require a change of sign on the denominator when the
power is even.


So do we:

1. return the canonical form 0 / 1 when the result is zero?

2. return the appropriate instance that is zero when the result is zero?

3. maintain the sign as if implemented using the arithmetic on the
denominator?


I do not see a situation where preserving the signedness of the zero
denominator is important. This rules out 3.
But didn't you imply above that
   0 / -1   pow   2
should return
   0 / 1
?

If you computed pow(2) it as (0 * 0) / (-1 * -1) then the result is 0 / 1. Likewise pow(3) is 0 / -1. But both are zero.

If you see the result as zero then the sign of the denominator does not matter.

If you want to preserve the sign of the denominator then more work will have to be done in certain methods. I do not think this is important. Note: This is in contrast to Complex where the sign of zero components of the complex number have to be preserved to ensure compatibility with ISO C99 standards.

So do we leave it to produce 0 / -1 or 0 / 1 depending on the computation (as is done currently), fix it to preserve the sign appropriately or fix it to ensure that zero is always a canonical 0 / 1?



It makes it cleaner in the code to always return Fraction.ZERO when
operations result in zero. This has memory advantage by allowing the
garbage collection of fractions that are zero when they go out of scope
following computations.
+1

Returning 0 / 1 for zero when possible is simple to do in the existing methods that detect zero as one of the arguments.

This is a weak argument as it is unlikely to
save much memory in common usage. It is also possible to create zeros in
computation and we do not want to go the route of switching new 0/1
instances to ZERO.
Why not?
It is extra work in the computations since computations do not detect zeros in the result. They detect zeros in the arguments and can fast rack the result.

For those that actually compute a result then the result would require construction using a factory constructor that will detect and return ZERO if the numerator is zero. This would add another if statement to all usage of the factory constructor and effect performance. However performance impact is not likely to be high (see below).

It should solve the previous issue that you can represent zero as 0 / -1. If all computations use a factory constructor then we can make it possible to exclude the 0 / -1 representation completely.


The easiest change is to just update multiply to return the appropriate
zero instance and not use Fraction.ZERO. The classes then are consistent
across the methods. Return the appropriate fraction instance if it
matches the result, otherwise compute the result.
I don't follow: How do you match before computing?

I missed out that one of the function arguments match the result when zero is involved as an argument. All the arithmetic methods detect zeros in the arguments and quickly return.

It would be cleaner to try and enforce a canonical form for ZERO.

Looking at the code for the standard constructor using Fraction(num, den) there are already a few if statements in there. In most cases it then follows the path using the reduction with the greatest common divisor logic. This is bound to be the largest part of the execution time. Forcing all construction through the of(num, den) factory method and adding another if statement to detect a zero numerator should not impact performance too much. The class can avoid the factory constructor if the value has already been tested and known to be non zero. This is the case for methods that use a numerator and not a denominator (such as pow(int)).

Given this I am thinking that using ZERO when possible is a better option and avoid 0 / -1.


Also, did you have an opinion on dropping pow(long) and pow(BigInteger)? I do not think that they can compute a value when pow(int) cannot be used.



The ZERO constant then is reduced to a convenience as for the ONE constant.

So my vote is option 2 for code consistency.


Alex
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to