HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Fraction class in Java - follow-up

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
classfollowjavafraction

Problem

(See the previous iteration.)

I have improved my Fraction implementation taking the answers in the previous iteration into account.

Differences

  • Both numerator and denominator are now instances of BigInteger.



  • The class also caches a BigDecimal representing the value of the ratio with no more than scale decimals.



  • When constructing a Fraction, both the numerator and denominator are divided by their greatest common divisor. This is much shorter and efficient than in the previous iteration, where I dealt with prime factorization to do the same task.



  • Fraction is declared final.



  • toString() is simplified.



  • The check whether the new Fraction is positive or negative is simplified.



  • serialVersionUID is added.



  • Added two public constants for zero and one.



However, I did not like the static factory pattern, so it's not here.

See what I have:

Fraction.java:

```
package net.coderodde.math;

import java.math.BigDecimal;
import java.math.BigInteger;
import java.math.RoundingMode;
import java.util.Objects;

/**
* This class implements a fraction consisting of a numerator and a denominator.
*/
public final class Fraction extends Number {

public static final Fraction ZERO = new Fraction(0, 1);
public static final Fraction ONE = new Fraction(1, 1);

private static final int DEFAULT_SCALE = 5;
private static final long serialVersionUID = 185726081998L;

private final BigInteger numerator;
private final BigInteger denominator;
private final BigDecimal value;

private Fraction(BigInteger numerator,
BigInteger denominator,
final int scale) {
Objects.requireNonNull(numerator, "The input numerator is null.");
Objects.requireNonNull(denominator, "The input denominator is null.");

if (denominator.equals(BigInteger.ZERO)) {
throw new IllegalArgumentException("The denominator is zero.");
}

if (numerator.equals(BigIntege

Solution

Introducing a BigInteger is not a good idea. It does solve the problem of potential overflows during calculations but it comes with a big cost. BigInteger over primitives slows down performance a lot and it complicates the code. I would have had two classes:

  • a Fraction with long numerator and denominator.



  • a BigFraction with BigInteger numerator and denominator.



To test this, I made a JMH benchmark comparing the performance of a Fraction and a BigFraction. Fraction is based on your initial class, i.e. keeping long values as numerator and denominator, and it implements a couple of simplifications (like the greatest common divisor). BigFraction is the class in this post (where I just renamed it). The benchmark creates 10, 100, 1.000 and 10.000 fractions of both classes, with the same numerator and denominator chosen at random, and measures the time it takes to sum them all together.

The results speak for themselves (JDK 1.8.0_74, Windows 10 64 bits, i5, CPU @ 2.90 Ghz):

Benchmark               (length)  Mode  Cnt   Score    Error  Units
StreamTest.bigFraction        10  avgt   30   0,059 ±  0,003  ms/op
StreamTest.bigFraction       100  avgt   30   0,623 ±  0,025  ms/op
StreamTest.bigFraction      1000  avgt   30   6,583 ±  0,340  ms/op
StreamTest.bigFraction     10000  avgt   30  64,364 ±  2,081  ms/op
StreamTest.fraction           10  avgt   30   0,005 ±  0,001  ms/op
StreamTest.fraction          100  avgt   30   0,049 ±  0,001  ms/op
StreamTest.fraction         1000  avgt   30   0,506 ±  0,008  ms/op
StreamTest.fraction        10000  avgt   30   5,025 ±  0,069  ms/op


Using BigFraction is more than 12 times slower than using Fraction, for exactly the same numbers.

For completeness, this is the JMH benchmark:

@Warmup(iterations = 10, time = 700, timeUnit = TimeUnit.MILLISECONDS)
@Measurement(iterations = 10, time = 700, timeUnit = TimeUnit.MILLISECONDS)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@Fork(3)
public class StreamTest {

    @State(Scope.Benchmark)
    public static class FractionContainer {

        @Param({ "10", "100", "1000" })
        private int length;

        private List bigFractions;
        private List fractions;

        @Setup(Level.Iteration)
        public void setUp() {
            ThreadLocalRandom random = ThreadLocalRandom.current();
            long numerator = random.nextLong(), denominator = random.nextLong();
            fractions = IntStream.range(0, length).mapToObj(i -> new Fraction(numerator, denominator)).collect(Collectors.toList());
            bigFractions = IntStream.range(0, length).mapToObj(i -> new BigFraction(numerator, denominator)).collect(Collectors.toList());
        }

    }

    @Benchmark
    public Fraction fraction(FractionContainer container) {
        return container.fractions.stream().reduce(new Fraction(0, 1), Fraction::plus);
    }

    @Benchmark
    public BigFraction bigFraction(FractionContainer container) {
        return container.bigFractions.stream().reduce(new BigFraction(0, 1), BigFraction::plus);
    }

}

Code Snippets

Benchmark               (length)  Mode  Cnt   Score    Error  Units
StreamTest.bigFraction        10  avgt   30   0,059 ±  0,003  ms/op
StreamTest.bigFraction       100  avgt   30   0,623 ±  0,025  ms/op
StreamTest.bigFraction      1000  avgt   30   6,583 ±  0,340  ms/op
StreamTest.bigFraction     10000  avgt   30  64,364 ±  2,081  ms/op
StreamTest.fraction           10  avgt   30   0,005 ±  0,001  ms/op
StreamTest.fraction          100  avgt   30   0,049 ±  0,001  ms/op
StreamTest.fraction         1000  avgt   30   0,506 ±  0,008  ms/op
StreamTest.fraction        10000  avgt   30   5,025 ±  0,069  ms/op
@Warmup(iterations = 10, time = 700, timeUnit = TimeUnit.MILLISECONDS)
@Measurement(iterations = 10, time = 700, timeUnit = TimeUnit.MILLISECONDS)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@Fork(3)
public class StreamTest {

    @State(Scope.Benchmark)
    public static class FractionContainer {

        @Param({ "10", "100", "1000" })
        private int length;

        private List<BigFraction> bigFractions;
        private List<Fraction> fractions;

        @Setup(Level.Iteration)
        public void setUp() {
            ThreadLocalRandom random = ThreadLocalRandom.current();
            long numerator = random.nextLong(), denominator = random.nextLong();
            fractions = IntStream.range(0, length).mapToObj(i -> new Fraction(numerator, denominator)).collect(Collectors.toList());
            bigFractions = IntStream.range(0, length).mapToObj(i -> new BigFraction(numerator, denominator)).collect(Collectors.toList());
        }

    }

    @Benchmark
    public Fraction fraction(FractionContainer container) {
        return container.fractions.stream().reduce(new Fraction(0, 1), Fraction::plus);
    }

    @Benchmark
    public BigFraction bigFraction(FractionContainer container) {
        return container.bigFractions.stream().reduce(new BigFraction(0, 1), BigFraction::plus);
    }

}

Context

StackExchange Code Review Q#127125, answer score: 4

Revisions (0)

No revisions yet.