Details
-
Improvement
-
Status: Closed
-
Minor
-
Resolution: Fixed
-
3.5
-
None
-
None
-
Patch
Description
In many cases a BigFraction object may represent a fraction in its most reduced form. Despite this, the reduce() method doesn't bother checking whether the BigFraction needs to be reduced before returning a new object.
This is problematic because equals() calls reduce() on both operands. This means that doing a simple comparison like BigFraction.ONE.equals(BigFraction.TWO) will result in two object allocations.
Additionally, while there is a very minimal attempt at optimizing around operations that take in a parameter of BigFraction.ZERO, it is somewhat inconsistent and incomplete. For example, BigFraction add(final BigFraction fraction) checks to see whether the incoming fraction is zero and if so, returns this. Why doesn't it also check whether this is zero and return the incoming fraction if so, avoiding an allocation?
This is one example, but it exists in other locations where we not only end up with allocations but expensive arithmetic operations as well (multiplication or exponentiation).
At minimum I think the reduce() method should just return this if gcd <= 1.
I attached a patch with the change for reduce() and some ideas for avoiding object allocations, attempting to follow the patterns in BigInteger. I verified all of the tests in the commons math pass after the changes.