Manipulating Common Fractions

I'm a beginner Java student currently learning from Absolute Java 5th 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 ze

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.

StackExchange Code Review Q#54175, answer score: 5