Pergunta

I have written a Fraction class, and am having trouble with the simplification.

When I make the Fraction object, everything works fine, i just think my logic is messy with the simplification.

(num and den are private variables in the class for numerator and denominator respectively)

Here are my GCD and Simplify methods:

/**
 * Returns the absolute value of the greatest common divisor of this
 * fraction's numerator and denominator. If the numerator or denominator is
 * zero, this method returns 0. This method always returns either a positive
 * integer, or zero.
 * 
 * @return Returns the greatest common denominator
 */
private int gcd() {
    int s;
    if (num > den)
        s = den;
    else
        s = num;
    for (int i = s; i > 0; i--) {
        if ((num % i == 0) && (den % i == 0))
            return i;
    }
    return -1;
}

/**
 * Changes this fraction's numerator and denominator to "lowest terms"
 * (sometimes referred to as a "common fraction"), by dividing the numerator
 * and denominator by their greatest common divisor. This includes fixing
 * the signs. For example, if a fraction is 24/-18, this method will change
 * it to -4/3. If the numerator or denominator of the fraction is zero, no
 * change is made.
 */
public void simplify() {

    if (isZero() == false) {// Making sure num or den is not zero.
        this.fixSigns(); // Fix signs first

        if (gcd() > 1) {
            this.num = num / gcd();
            this.den = num / gcd();
        }
    }
}
Foi útil?

Solução

Two things I see right away: You are dividing num by gcd() twice, for each of the numerator and denominator. Also, once, you change the numerator, then the result of the call to gcd() may change. Call "gcd" once, store its result, and use it later:

int gcd = gcd();
if (gcd > 1) {
   this.num = this.num / gcd;
   this.den = this.den / gcd;
}

Additionally, there are more efficient ways of obtaining the greatest common divisor: Wikipedia's page. See Euclid's algorithm on that page.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top