Fraction class in Java

(See the next iteration.)

I have this class for representing exact fractions. See what I have:

Fraction.java:

```
package net.coderodde.math;

import java.util.ArrayList;
import java.util.List;

/**
* This class implements a fraction consisting of a numerator and a denominator.
*
* @author Rodion "rodde" Efremov
* @version 1.6 (Apr 29, 2016)
*/
public class Fraction extends Number {

private final long numerator;
private final long denominator;

public Fraction(final long numerator, final long denominator) {
if (denominator == 0) {
throw new IllegalArgumentException("The denominator is zero.");

}

if (numerator == 0) {
this.numerator = 0;
this.denominator = 1;
} else {
final boolean isPositive = isPositive(numerator, denominator);
final long[] data = eliminateCommonFactors(numerator, denominator);

this.numerator = isPositive ? data[0] : -data[0];
this.denominator = data[1];
}
}

public Fraction plus(final Fraction other) {
return new Fraction(this.numerator * other.denominator +
this.denominator * other.numerator,
this.denominator * other.denominator);
}

public Fraction minus(final Fraction other) {
return new Fraction(this.numerator * other.denominator -
this.denominator * other.numerator,
this.denominator * other.denominator);
}

public Fraction multiply(final Fraction other) {
return new Fraction(this.numerator * other.numerator,
this.denominator * other.denominator);
}

public Fraction divide(final Fraction other) {
return new Fraction(this.numerator * other.denominator,
this.denominator * other.numerator);
}

public Fraction abs() {
return new Fraction(Math.abs(numerat

You can make a lot of simplifications.

Non zero numbers with the same sign

Currently, you have a method that tests whether two given non-zero long values have the same sign with the following:

private boolean isPositive(final long numerator, final long denominator) {
    final boolean numeratorIsPositive = numerator > 0L;
    final boolean denominatorIsPositive = denominator > 0L;

    return (numeratorIsPositive && denominatorIsPositive) || (!numeratorIsPositive && !denominatorIsPositive);
}

This can be written more concisely with:

private boolean isPositive(final long numerator, final long denominator) {
    return numerator > 0 == denominator > 0;
}

-
You don't need to have L suffix. It is guaranteed that the check will be done on long values due to binary numeric promotion . JLS section 15.20.1 Numerical Comparison Operators `, and >=:


Binary numeric promotion is performed on the operands (§5.6.2).

and JLS section 5.6.2:


Otherwise, if either operand is of type long, the other is converted to long.

So, rest assured.

-
You can simply check whether the fact that both numbers are greater than 0 is the same: if they are both greater or lower than 0 then the result will be true. Note that both numbers can't be equal to 0 since that case was already handled.

As such, you may not even need that method and directly have:

final boolean isPositive = numerator > 0 == denominator > 0;

Simplifying a fraction

The biggest complication is your method to simplify a fraction. You currently have a complicated way with determining the prime factor when you can do it in a much simpler way. Just calculate the greatest common divisor of both the numerator and the denominator.

private static long gcm(long a, long b) {
    return b == 0 ? a : gcm(b, a % b);
}

Then your code in the constructor simply becomes:

final boolean isPositive = numerator > 0 == denominator > 0;
final long gcm = gcm(numerator, denominator);

this.numerator   = isPositive ? Math.abs(numerator / gcm) : -Math.abs(numerator / gcm);
this.denominator = Math.abs(denominator / gcm);

All of your tests still pass with this change.

toString()

Small nitpick, in the following:

@Override
public String toString() {
    return "" + numerator + "/" + denominator;
}

you don't need the empty string. Just have:

@Override
public String toString() {
    return numerator + "/" + denominator;
}

No serialVersionUID

You are extending from Number which is serializable; this makes your class serializable also. As such, you should defined a serial version UID for your class.

Final and immutable classes

Do you intend to override that class in the future? It doesn't look like a good idea to do it. I would make that class final to make it impossible, just like the built-in Integer or Long.

The fact that the class is immutable is a very good idea.

Static factories

Consider creating a pool of common fractions, like ONE or ZERO. Then you can create static factories to return Fraction instances instead of using the constructor directly.

Typically, this is done by introducing a method valueOf(...) that will return the instance of Fraction. You can take inspiration from Integer.valueOf or BigDecimal.valueOf. The constructor is then made private` so that the caller needs to go through that method.

As an example you could create two public constants for zero and one:

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

Then, you can reuse them in the static factory:

public static Fraction valueOf(final long numerator, final long denominator) {
    if (denominator == 0) {
        throw new IllegalArgumentException("The denominator is zero.");
    }
    if (numerator == 0) {
        return ZERO;
    } else if (numerator == denominator) {
        return ONE;
    }
    return new Fraction(numerator, denominator);
}

This avoids to instantiate new fractions and reuse existing ones.

StackExchange Code Review Q#127028, answer score: 15