Java Fraction Calculator

I validate this program for

Non numeric inputs
x/0 type fractions
0/0 and 0/y type fractions
x/1 and x/x type fractions
-x/-y and x/-y  type fractions

I'd appreciate it if someone can look at this code and tell where I can improve this coding. I'd also like to know about other validations best practices I can use or contribute on Github.

IDE used: Eclipse (Kepler)

Blog post

Github repositary

FractionsApp.java

```
package com.gigal.fractionexercise.app;

import java.util.Scanner;

import com.gigal.fractionexercise.helper.Messages;
import com.gigal.fractionexercise.model.Division;
import com.gigal.fractionexercise.model.Fraction;
import com.gigal.fractionexercise.model.Multiplication;
import com.gigal.fractionexercise.model.Subtraction;
import com.gigal.fractionexercise.model.Addition;

public class FractionsApp {

private static Scanner keyboard = new Scanner(System.in);

public static void main(String args[]) {

Fraction fraction1 = new Fraction(); // first fraction
Fraction fraction2 = new Fraction(); // second fraction

// Display application header
Messages.displayHeader();

// get user inputs for fraction one and validate them
do {
System.out.println("Enter values for fration one");
Messages.insertNumerator();
try {
fraction1.setNumerator(keyboard.nextInt()); // get user input
} catch (Exception e) {
Messages.inputError(e); // display error
return;
}
Messages.inputDenominator();
try {
fraction1.setDenominator(keyboard.nextInt()); // get user input
} catch (Exception e) {
Messages.inputError(e);
return;
}
if (fraction1.getDenominator() == 0) { // check for x/0 error
Messages.DenominatorCannotBeZero();
}
} while (fraction1.getDenominator() == 0);

// Display fraction one
System.out.print("Fraction one : ");
fraction1.display();
Messages

There are several things that can be improved in your code, I will point out some of them here.

Polymorphism

Your Addition/Subtraction/Multiplication/Division classes has a lot in common. You should use polymorphism and inheritance to use them better. You can make an abstract class for them.

Also, you should make your unchangeable Fraction fields final. Also, the method Calculate should be named calculate to comply with the Java coding conventions. This would also apply to methods such as DenominatorCannotBeZero. All methods should start with lower-case letter. The same goes for Numerator and Denominator in your Fraction class.

As your Messages.displayAnswer method was only called from the calculation classes, I put that code in this method instead.

public abstract class Calculation {
   private final Fraction fraction1;
   private final Fraction fraction2;
   private final String operation;
   private final char operator;
   protected Fraction answer;

   public Calculation(Fraction fraction1, Fraction fraction2, String operation, char operator) {
       this.fraction1 = fraction1;
       this.fraction2 = fraction2;
       this.operation = operation;
       this.operator = operator;
       this.answer = new Fraction();
       calculate();
   }

   public abstract void calculate();

   public void displayAnswer() {
       System.out.print(operation + "  : ");
       fraction1.display();
       System.out.print(" " + operator + " ");
       fraction2.display();
       System.out.print(" = ");
       answer.display();
       System.out.println("");
       System.out.println("");
   }
}

// Example with the Addition class
public class Addition extends Calculation {
     public Addition(Fraction fraction1, Fraction2) {
         super(fraction1, fraction2, "Addition", '+');
     }
     public void calculate() {
         answer.setNumerator((fraction1.getNumerator() * fraction2.getDenominator())
        + (fraction2.getNumerator() * fraction1.getDenominator()));
         answer.setDenominator(fraction1.getDenominator() * fraction2.getDenominator());
     }
}

Your Messages class

Messages.insertNumerator(); is only called when you are asking the user to input some value. Move the entire input code to this method, you could also let the method take care of the error-handling, this way you would only have to call:

fraction1.setNumerator(Messages.inputNumerator());

Overall, I think you are overusing your Messages, or in one way underusing. You are using it to reduce code-duplication, and yet you still have code duplication. Once you put the input for both inputNumerator and inputDenominator into your Messages classes, I would agree with the usage of it more. However, a method for new-line is a bit overkill IMO. I personally would think it is more clear to actually print System.out.println(); once or twice when you want an empty line.

Other suggestions

Instead of what I have done above, you could use a CalculationResult class to store the answer and let it have the displayAnswer method.

Instead of creating the Calculation on one line and displaying it on the next and then never using that variable again, you can use this:

new Addition(fraction1, fraction2).display();

No variable created, you just create the object and use it directly.

A comment like this is completely overkill. Make your code as self-documenting as possible. The variable name addition tells you that it is addition.

// Addition
Addition addition = ...

StackExchange Code Review Q#43084, answer score: 13