2020-04-09 12:33 UTC+02:00, Alex Herbert <alex.d.herb...@gmail.com>: > > On 09/04/2020 10:54, Gilles Sadowski wrote: >> Hi. >> >> 2020-04-09 11:28 UTC+02:00, Alex Herbert <alex.d.herb...@gmail.com>: >>> Two oddities in fraction: >>> >>> 1. Zero can be represented as 0 / 1 or 0 / -1 >>> >>> I have not found any location where this manifest as a bug. >> :-) >> >>> It is >>> handled by the arithmetic and toString methods and the denominator is >>> not negated in the method negate() and so negation of 0 / -1 returns the >>> same value: >>> >>> Fraction f1 = Fraction.of(0, -1); >>> Fraction f2 = f1.negate(); >>> System.out.printf("%s/%s (%s) : %s/%s (%s)%n", >>> f1.getNumerator(), f1.getDenominator(), f1, >>> f2.getNumerator(), f2.getDenominator(), f2); >>> >>> 0/-1 (0) : 0/-1 (0) >>> 0/-1 (0) : 0/-1 (0) >>> >>> However any future issues could be avoided by detecting a zero numerator >>> in the constructors and setting the denominator as 1. >> Wouldn't this mean a performance hit for the majority of >> instances that are not zero? > > Yes, but the difference may be marginal. > > 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.
+1 [We removed the requirement that the denominator must be positive. It would have been strange to reinstate it for the case where it least matters.] >> >>> 2. There are a lot of pow(exponent) methods in BigFraction compared to >>> Fraction: >>> >>> This is mandated by o.a.c.numbers.core.NativeOperations: >>> >>> Fraction pow(int) >>> >>> BigFraction pow(int) >>> >>> But there are more in BigFraction and one returns a double: >>> >>> BigFraction pow(long) >>> BigFraction pow(BigInteger) >>> double pow(double) >>> >>> I see the long and BigInteger versions as specialisations of the >>> pow(int) method. The double variant can be implemented in Fraction using >>> the same method from BigFraction as: >>> >>> /** >>> * Returns a {@code double} whose value is >>> * <code>this<sup>exponent</sup></code>. >>> * >>> * @param exponent exponent to which this {@code Fraction} is to >>> be >>> raised. >>> * @return <code>this<sup>exponent</sup></code> as a {@code >>> double}. >>> */ >>> public double pow(final double exponent) { >>> return Math.pow(numerator, exponent) / >>> Math.pow(denominator, exponent); >>> } >> This now strikes me as dangerous. >> >>> So either we add this method to Fraction >> -1 >> >>> or drop it from BigFraction. >> +1 >> >> Let users perform any "cast" to "double" that could be invalid. > > OK. > > I was wondering about: > > BigFraction pow(long) > BigFraction pow(BigInteger) > > These are implemented by computing: > > pow(numerator, exponent) and pow(denominator, exponent) and then > creating a fraction from the result which will perform a reduction. When > the exponent absolutely must be a long or BigInteger (and not an int) > then I imagine that this would overflow BigInteger. On my machine this: > > BigInteger.valueOf(2).pow(Integer.MAX_VALUE) > > reports this: > > java.lang.ArithmeticException: BigInteger would overflow supported range Strange; this doesn't seem to be consistent with the Javadoc:[1] ---CUT--- Semantics of arithmetic operations exactly mimic those of Java's integer arithmetic operators, as defined in The Java Language Specification. For example, division by zero throws an ArithmeticException, and division of a negative by a positive yields a negative (or zero) remainder. All of the details in the Spec concerning overflow are ignored, as BigIntegers are made as large as necessary to accommodate the results of an operation. ---CUT--- > > So I think we can drop the pow() methods using a long or BigInteger > argument too. In cases where you have to use them they will overflow. > >> >>> Note: The methods are present in the CM 3.6 version of BigFraction but >>> Fraction did not have any pow methods. >> A lot of fixes and improvements were brought in by >> Heinrich Bohne a few months ago. [IIRC, some issues >> which he had reported are still pending...] > I remember the good work on the constructors and conversion to/from > double. I did not look at reported issues. I was focussing on the > current functionality. I didn't mean any subliminal message, except for Heinrich maybe. ;-) Regards, Gilles [1] https://docs.oracle.com/javase/8/docs/api/java/math/BigInteger.html --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org