[FIXED] Method simplify(Fraction f) does not work

Issue

I have a class Calculator which aggregates instances of a class Fraction as its attributes.

Class Fraction has attributes num for numerator and denom for denominator.

Here is an abstract of the code with 'multiply' and 'simplify' (to get a fraction in its lowest terms) methods.

public class Calculator {
    private Fraction f1 = new Fraction(4, 9);
    private Fraction f2 = new Fraction(3, 8);

    public void multiply() throws Exception {
        int num = f1.getNum() * f2.getNum();
        int denom = f1.getDenom() * f2.getDenom();
        Fraction f = new Fraction(num, denom);
        simplify(f);
        System.out.print(f);
    }

  private void simplify(Fraction f) throws Exception {
        int num = f.getNum();
        int denom = f.getDenom();
        for (int i = num; i > 0; i--) {
            if ((num % i == 0) && (denom % i == 0)) {
                num = num / i;
                denom = denom / i;
                break;
            }
        }
    }

However, I get 12/72 as a result of multiplication while I should get 1/6. How can I change the code so that 'simplify' method works when invoked in 'multiply'?

Solution

As Edwin commented you want algorithm for greatest common divisor. However to answer your question, you got unexpected result because the newly computed variables num and denom at the line with num = num / i are not stored back into Fraction f.

Either (worse option) call f.setNum(num); f.setDenom(denom) or (better) change Fraction to immutable class and return new Fraction from simplify method.

Answered By - Tomáš Záluský
Answer Checked By - Gilberto Lyons (JavaFixing Admin)