Manipulating Common Fractions

Question

I'm a beginner Java student currently learning from Absolute Java 5^th edition. I think I understand the content well but I am never sure of my style. This code is for a question after the second chapter on defining classes. I haven't learned arrays or recursion yet.

• Am I following or close to best coding practices?
• Am I committing any serious style errors?
• Is my decomposition OK?
• Readability?

The textbook question is written in the opening comments.

/*
 * Define a class called Fraction. This class is used to represent a ratio of two integers. 
 * Include mutator methods that allow the user to set the numerator and the denominator. 
 * Also include a method that displays the fraction on the screen as a ratio (e.g., 5/9). 
 * This method does not need to reduce the fraction to lowest terms. 
 * 
 * Include an additional method, equals, that takes as input another Fraction and returns 
 * true if the two fractions are identical and false if they are not. This method should 
 * treat the fractions reduced to lowest terms; that is, if one fraction is 20/60 and the
 * other is 1/3, then the method should return true.
 * 
 * Embed your class in a test program that allows the user to create a fraction. Then the 
 * program should loop repeatedly until the user decides to quit. Inside the body of the 
 * loop, the program should allow the user to enter a target fraction into an anonymous 
 * object and learn whether the fractions are identical. 
 */

import java.util.Scanner;

public class FractionClass {
    int num, den;
    public FractionClass(int startingNum, int startingDen) {
        if (startingDen == 0) {
            System.out.println("Error. Denominator cannot equal zero.");
            System.exit(1);
        }
        num = startingNum;
        den = startingDen;
    }

    // Mutator method to set the numerator
    public void setNum(int newNum) {
        num = newNum;
    }

    // Mutator method to set the denominator which cannot equal zero
    public void setDen(int newDen) {
        if (newDen == 0) {
            System.out.println("Error. Denominator cannot equal zero.");
            System.exit(1);
        }
        den = newDen;
    }

    // Displays the fraction in standard form as a String
    public String toString() {
        return (num + "/" + den); 
    }

    // Two FractionClass objects are considered equal if the reduced forms of each fraction are equal
    public boolean equals(FractionClass otherObject) {
        reduceFraction();
        otherObject.reduceFraction();
        boolean numEqual = (this.num == otherObject.num);
        boolean denEqual = (this.num == otherObject.num);
        return (numEqual && denEqual);
    }

    // Reduces the fraction using Euclid's Algorithm Method
    // Pre-Condictions: The calling object has a valid numerator and denominator
    // Post-Conditions: The calling object's numerator and denominator are reduced
    private void reduceFraction() {
        int GCD = euclidGCD(num, den);
        num = num / GCD;
        den = den / GCD;
    }

    // Euclid's Algorithm. Takes two integers and returns the greatest common divisor. 
    private int euclidGCD(int a, int b) {
        while (a != 0 && b != 0) {
            int c = b;
            b = a%b;
            a = c;
        }
        return a+b;
    }

    private static final String EXIT_KEYWORD = "exit"; // Keyword to end the main program loop

    public static void main(String[] args) {
        int numInput, denInput;
        Scanner kb = new Scanner(System.in);

        boolean resume = true;        
        boolean fractionsEqual;
        while (resume) {
            numInput = readNum(kb);
            denInput = readDen(kb);
            FractionClass firstFraction = new FractionClass(numInput, denInput);
            System.out.println("You entered the fraction: " + numInput + "/" + denInput);
            System.out.println("Now we will compare that to a new fraction.");
            numInput = readNum(kb);
            denInput = readDen(kb);
            fractionsEqual = firstFraction.equals(new FractionClass(numInput, denInput));
            if (fractionsEqual) {
                System.out.println("The fractions are equal.");
            } else {
                System.out.println("The fractions are not equal.");
            }

            resume = shouldResume(kb);
        }
    }

    // Reads an integer numerator and returns the value
    private static int readNum(Scanner kb) {
        System.out.println("Enter a numerator: ");
        return kb.nextInt();
    }

    // Reads a non zero integer denominator and returns the value
    private static int readDen(Scanner kb) {
        boolean incorrectInput = true;
        int denInput = 1;
        while (incorrectInput) {
            System.out.println("Please enter a denominator that doe not equal zero ");
            denInput = kb.nextInt();
            if (denInput != 0) incorrectInput = false;
        }
        return denInput;
    }


    // Asks the user if they want to run the program again
    // Returns false if the user input matches the exit keyword defined as a constant otherwise returns true
    private static boolean shouldResume(Scanner kb) {
        String resumeAnswer = "";
        System.out.println("Please enter \"" + EXIT_KEYWORD + "\" to exit the program or enter any key to run the program again.");
        resumeAnswer = kb.next();
        resumeAnswer = resumeAnswer.toLowerCase();
        if (resumeAnswer.equals(EXIT_KEYWORD)) {
            return false;
        } else {
            return true;
        }
    }
}

Answer 1

You don't really need both readNum() and readDen() methods, with the latter performing input validation. What you can consider is to take two integer inputs and pass them into a static factory method that will do the validation for you, and throwing an Exception if necessary.

public static Fraction newInstance(int numerator, int denomiator) {
    if (denominator == 0) {
        throw new IllegalArgumentException("Denominator cannot be zero.");
    }
    return new Fraction(numerator, denominator);
}

The other more glaring problem lies in your equals() method, I will be quite concerned when it actually modifies the state of my compared object by calling its reduceFraction() method. To better resolve this bug, perhaps you need to ask yourself:

• When I create a new Fraction object, can I immediately call reduceFraction() to simplify the representation?
• Should reduceFraction() really modify the object's state, or should I preserve immutability and return a new Fraction object?
• (advanced) If I am preserving immutability, do I also want to compute a hashcode value for my Fraction objects so that future comparisons can be done much faster and simpler?

For what it's worth, the method signature to over-ride should really be equals(Object obj) (with the necessary check for the correct type), and you are also recommended to over-ride the hashCode() method.

public boolean equals(Object obj) {
    return obj instanceof Fraction && <test for values>; // simple example
}

Here's more information regarding equals() and hashCode() straight from Javadoc. :)

Answer 2

A few things that I would change (in rough order of importance):

• Rename class to Fraction (as mentioned by Debasis)
• Throw an IllegalArgumentsException in the constructor, instead of calling print and exit(1). Users who use your class wrongly will then get a stack trace. This contains context, e.g., where in their code they created the illegal fraction.
• I would prefer to keep the fraction reduced at all times, rather than reducing it when calling equals. It will be very confusing for people if the object is different before and after an equality comparison.
• The equals method is overridden from Object (the base class of all java classes). Thus it should have the same signature: boolean equals(Object other). Also, overriding equals means you should also override hashCode (Otherwise your object will behave strangely when used in data structures).
• Do you really need to consider the case where b==0 in your GCD implementation?
• Instead of just readNum and readDen, you could create a readFraction function to save yourself some repetition.
• I usually call my constructor arguments the same as the class attributes, and use this.num = num. People differ on this, though...

Another remark: You're explicitly asked to create mutator functions. However, the code would be much more robust and simple if a Fraction were immutable. This would mean final num and den attributes (they could even be public). You would only need to check for the zero denominator and reduce the fraction in a single place (the constructor).

If you want to make your fraction really robust, there are a number of other things you should consider:

• should -1/-5 be simplified to 1/5?
• if yes, be careful about -1/Integer.MIN_VALUE, because negating Integer.MIN_VALUE does not do what one thinks it does.

Answer 3

General Remark: You have developed a good sense of object oriented approach. Carry on with your good work...

Specific comments:

1. Change the name FractionClass to Fraction

2. You have defined the toString() method for your fraction data type (class). Yet, while printing an object you have simply used:

   println(numInput + "/" + denInput)
   

   You simply need to call the toString() method for this.