Java Fraction Calculator

Question

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.newLine();

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

    // Display fraction two
    System.out.print("Fraction two : ");
    fraction2.display();
    Messages.newLine();

    // Addition
    Addition addition = new Addition(fraction1, fraction2);
    addition.display();

    // Subtraction
    Subtraction subtraction = new Subtraction(fraction1, fraction2);
    subtraction.display();

    // Multiplication
    Multiplication multiplication = new Multiplication(fraction1, fraction2);
    multiplication.display();

    // Division
    Division division = new Division(fraction1, fraction2);
    division.display();

    // Display application footer
    Messages.displayFooter();
}

}

Messages.java

package com.gigal.fractionexercise.helper;

import com.gigal.fractionexercise.model.Fraction;

/*
 * This class is used to give meaningful messages to user
 * So whenever we want to change a message we don't want to check whole app but this class
 */

public class Messages {

// This message is use to display the program header
public static void displayHeader() {
    System.out.println("_________________________________________________________");
    System.out.println("         Fraction App - a Gigal Blog Production");
    System.out.println("_________________________________________________________");
    newLine();
}

// This message is use to display the program footer
public static void displayFooter() {
    newLine();
    System.out.println(" ----------- Thank you for using Fraction App -----------");
    System.out.println("_________________________________________________________");
    newLine();
}

// This message is use to tell user to input value for Numerator in a fraction
public static void insertNumerator() {
    System.out.print("Numerator : ");
}

// This message is use to tell user to input a value for denominator
public static void inputDenominator() {
    System.out.print("Denominator   : ");
}

// This method is used to get line of space
public static void newLine() {
    System.out.println("\n");
}

// This message is used when user input something miss match
public static void inputError(Exception e) {
    newLine();
    System.out.println("Input Error: " + e.toString());
    System.out.println("Closing application ...");
    System.out.println("Fraction app is closed.");
    displayFooter();
}

// This message is used when user input 0 for Denominator in a fraction
public static void DenominatorCannotBeZero() {
    System.out.println("Input Error: Denominator Cannot be zero");
    newLine();
}

// This message is used to display answers
public static void displayAnswer(String operation, String operator,
        Fraction fraction1, Fraction fraction2, Fraction answer) {
    System.out.print(operation + "  : ");
    fraction1.display();
    System.out.print(" " + operator + " ");
    fraction2.display();
    System.out.print(" = ");
    answer.display();
    newLine();
}

}

Addition.java

package com.gigal.fractionexercise.model;

/*
 * This class models addition
 */

import com.gigal.fractionexercise.helper.Messages;

public class Addition {

private Fraction fraction1;
private Fraction fraction2;
private Fraction answer;

// Constructor
public Addition(Fraction fraction1, Fraction fraction2) {
    this.fraction1 = fraction1;
    this.fraction2 = fraction2;
    this.answer = new Fraction();
    Calculate();
}

// perform the calculation
public void Calculate() {
    answer.setNumerator((fraction1.getNumerator() * fraction2.getDenominator())
            + (fraction2.getNumerator() * fraction1.getDenominator()));
    answer.setDenominator(fraction1.getDenominator() * fraction2.getDenominator());
}

// display the answer
public void display() {
    Messages.displayAnswer("Addition", "+", fraction1, fraction2, answer);
}

}

Division.java

package com.gigal.fractionexercise.model;

/*
 * This class models division
 */

import com.gigal.fractionexercise.helper.Messages;

public class Division {

private Fraction fraction1;
private Fraction fraction2;
private Fraction answer;

// Constructor
public Division(Fraction fraction1, Fraction fraction2) {
    this.fraction1 = fraction1;
    this.fraction2 = fraction2;
    this.answer = new Fraction();
    Calculate();
}

// perform the calculation
public void Calculate() {
    answer.setNumerator(fraction1.getNumerator() * fraction2.getDenominator());
    answer.setDenominator(fraction1.getDenominator() * fraction2.getNumerator());
}

public void display() {
    // Check for the divide by zero error
    if (fraction2.getNumerator() == 0) {
        System.out.println("Division : Cannot divide by zero!");
        return;
    } else {
        // display the answer
        Messages.displayAnswer("Division", "/", fraction1, fraction2,
                answer);
    }
}

}

Fraction.java

package com.gigal.fractionexercise.model;

/*
 * This class models the fraction
 */

public class Fraction {

private int Numerator; // x
private int Denominator; // y

public int getNumerator() {
    return Numerator;
}

public void setNumerator(int Numerator) {
    this.Numerator = Numerator;
}

public int getDenominator() {
    return Denominator;
}

public void setDenominator(int Denominator) {
    this.Denominator = Denominator;
}

// This method is used to display fractions
// Some kind of processing also
public void display() {

    // 0/y and x/1 types
    if (Numerator == 0 || Denominator == 1) {
        System.out.print(Numerator);
    }

    // -x/-y and x/-y types
    else {
        if ((Numerator < 0 && Denominator < 0) || (Numerator > 0 && Denominator < 0)) {
            Numerator *= -1;
            Denominator *= -1;
        }

        // x/x type
        if (Numerator == Denominator) {
            System.out.print(Numerator);
            return;
        }

        System.out.print(this.Numerator + "/" + this.Denominator);
    }

}

}

Multiplication.java

package com.gigal.fractionexercise.model;

/*
 * This class models multiplication
 */

import com.gigal.fractionexercise.helper.Messages;

public class Multiplication {

private Fraction fraction1;
private Fraction fraction2;
private Fraction answer;

// Constructor
public Multiplication(Fraction fraction1, Fraction fraction2) {
    this.fraction1 = fraction1;
    this.fraction2 = fraction2;
    this.answer = new Fraction();
    Calculate();
}

// perform the calculation
public void Calculate() {
    answer.setNumerator(fraction1.getNumerator() * fraction2.getNumerator());
    answer.setDenominator(fraction1.getDenominator() * fraction2.getDenominator());
}

// display the answer
public void display() {
    Messages.displayAnswer("Multiplication", "*", fraction1, fraction2, answer);
}

}

Subtraction.java

package com.gigal.fractionexercise.model;

/*
 * This class models subtraction
 */

import com.gigal.fractionexercise.helper.Messages;

public class Subtraction {

private Fraction fraction1;
private Fraction fraction2;
private Fraction answer;

// Constructor
public Subtraction(Fraction fraction1, Fraction fraction2) {
    this.fraction1 = fraction1;
    this.fraction2 = fraction2;
    this.answer = new Fraction();
    Calculate();
}

// perform the calculation
public void Calculate() {
    answer.setNumerator((fraction1.getNumerator() * fraction2.getDenominator()) - (fraction2.getNumerator() * fraction1.getDenominator()));
    answer.setDenominator(fraction1.getDenominator() * fraction2.getDenominator());
}

// display the answer
public void display() {
    Messages.displayAnswer("Subtraction", "-", fraction1, fraction2, answer);
}

}

You say you validate for 5 cases. They beg to be automated tests. — abuzittin gillifirca, Mar 3, 2014 at 8:35

Simon Forsberg · Accepted Answer · 2014-02-28 19:50:46Z

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 = ...

@Simon Hi I am writing a code very similar to this, and am thinking as the abstract class is abstract, would it not mean that the abstract class can not be instantiated? Many Thanks — mp252, Aug 13, 2015 at 16:50

200_success · Accepted Answer · 2014-08-17 22:19:35Z

Fraction representation

@Simon has a good suggestion, which I'll elaborate on: Fraction should be immutable, for several reasons:

Reduce defensive copying: In a more complex application, you'll be passing Fractions around. If fractions are not immutable, then each function that you pass it to will want to make a defensive copy to make sure that it stays consistent even if the original object's value changes. For example, if you use a Fraction as a key in a HashMap, the HashMap could become inconsistent, because HashMap doesn't make a defensive copy.
Better constructor: It's good practice to create objects that are in a self-consistent, usable state. The default constructor is what I would call a half-assed constructor. In fact, new Fraction() produces an illegal value (the denominator is zero). It would be better to force users to call a Fraction(int numerator, int denominator) constructor.
Consistency: The Integer and BigDecimal classes in Java are also immutable. The language designers have thought it through; you should probably go with the flow.
Simpler notation: If the numerator and denominator were final, then you could consider making them public as well. (This suggestion may be controversial, and you may choose to ignore it.)

The Fraction class shouldn't be tied to System.out in any way. Instead of display(), implement toString().

Operator representation

All of your operators have something in common, and should implement a common interface or extend from a common base class.

You don't really want to the object to include its operands — you'd have to instantiate a new one for each calculation. You can make them singletons instead.

Suggestion (there are a lot of ideas here):

public interface FractionBinaryOperator {
    Fraction calculate(Fraction a, Fraction b);
}

public class FractionalDivision implements FractionBinaryOperator {
    /**
      * Singleton
      */
    public static final OPERATOR = new FractionalDivision();
    private FractionalDivision() {}

    Fraction calculate(Fraction dividend, Fraction divisor) {
        return FractionalMultiplication.OPERATOR.calculate(dividend, divisor.reciprocal());
    }
}

A more advanced approach, using anonymous inner classes:

public abstract class Operator {
    public abstract Fraction calculate(Fraction a, Fraction b);

    …

    public static final Operator MUL = new Operator() {
        public Fraction calculate(Fraction a, Fraction b) {
            return new Fraction(a.numerator * b.numerator, a.denominator * b.denominator);
        }
    };  

    public static final Operator DIV = new Operator() {
        public Fraction calculate(Fraction dividend, Fraction divisor) {
            return Operator.MUL.calculate(dividend, divisor.reciprocal());
        }
    };
}

public class FractionsApp {
    public static void main(String[] args) {
        …
        System.out.println(Operator.ADD.calculate(fraction1, fraction2));
        …
    }
}

Error handling

Division.calculate() shouldn't print a complaint when dividing by zero. Code that calculates should stick to calculating, and shouldn't concern itself with input/output. Instead, it should throw an exception, and let the caller decide how to handle it. In fact, if you don't check for a zero divisor, Java will naturally throw an ArithmeticException for you. You just have to have main() catch it and display a nicer message.

I agree with @200_success's error handling advice. But a few words need to be said to make a finer point. If you want a RuntimeException that you throw, such as ArithmeticException, be caught and handled by the callers of that method (including code yourself have written), you should declare that your method throws that exception. And If you want "division by zero" be handled differently from any other potential ArithmeticException ever you should throw some custom exception, which possibly extends ArithmeticException. — abuzittin gillifirca, Mar 3, 2014 at 8:25

zafarkhaja · Accepted Answer · 2014-03-04 09:29:19Z

While there are some good points in other answers, I would suggest something completely different. OOP is not only about Polymorphism and Inheritance towards which you intuitively designed your operation classes, although incompletely and it was pointed out by Simon André Forsberg. OOP is also about Encapsulation which is often neglected by many. Encapsulation simply says that you have to hide the details of your implementation behind the public interface. This has a good side effect which is one of the most important OOP features: it makes you keep your data and routines that operate on that data together in one place. Encapsulation is what differentiates OOP from Procedural programming. A related principle is called Tell, Don't Ask.

This said, a proper OOP-style design would be to convert the operation classes to methods of the Fraction class

public class Fraction {
    public void add(Fraction fraction) {}
    public void subtract(Fraction fraction) {}
    public void multiply(Fraction fraction) {}
    public void divide(Fraction fraction) {}
}

Another thing is that you should make use of a constructor other than the default one. The constructor will ensure that the newly created fraction is complete and good to go. What happens if someone tries to perform an operation on a fraction without setting the numerator and denominator first? This will force you to validate fractions before each operation. Constructor also is a good way to specify the required object properties

public class Fraction {
    public Fraction(int numerator, int denominator) {}
    public void add(Fraction fraction) {}
    public void subtract(Fraction fraction) {}
    public void multiply(Fraction fraction) {}
    public void divide(Fraction fraction) {}
}

Also, I agree with 200_success on that the Fraction class should be immutable

public class Fraction {
    public Fraction(int numerator, int denominator) {}
    public Fraction add(Fraction fraction) {}
    public Fraction subtract(Fraction fraction) {}
    public Fraction multiply(Fraction fraction) {}
    public Fraction divide(Fraction fraction) {}
}

PS. If you want to have a set of operations, Enum would be a better choice.

I think this is the most valuable piece of advice. All the other answers point out important aspects but creating and encapsulating abstract data types is very important — mariosangiorgio, Aug 17, 2014 at 21:57

Uri Agassi · Accepted Answer · 2014-03-04 11:32:18Z

DRY your code

In every place where you used copy+paste - you should check whether you could refactor it to re-use the code you've written. Aside from making the code shorter and easier on the eyes, it also makes it more maintainable, as if you need to fix something, you don't need to fix it for every copy of the code.

@Simon already addressed the Polymorphism of the Calaculate classes, but it is not all about OOP, for example, to prompt the user for the second fraction, you've copied the code from the first fraction. Your error handling there is also copied from one prompt to another - since you stop the program after each failure, you don't need to catch each one in flow - one catch at the end of the prompting it enough:

private Fraction promptForFraction(string name) {
    Fraction fraction = new Fraction();
    do {
        System.out.println("Enter values for fraction " + name);
        Messages.insertNumerator();
        fraction.setNumerator(keyboard.nextInt()); // get user input

        Messages.inputDenominator();
        fraction.setDenominator(keyboard.nextInt()); // get user input
        if (fraction.getDenominator() == 0) { // check for x/0 error
            Messages.DenominatorCannotBeZero();
        }
    } while (fraction.getDenominator() == 0);

    System.out.print("Fraction " + name + " : ");
    fraction.display();
    Messages.newLine();
    return fraction;
}

public static void main(String args[]) {

    // Display application header
    Messages.displayHeader();

    Fraction fraction1 = null;
    Fraction fraction2 = null;
    try {

        fraction1 = promptForFraction("one"); // first fraction
        fraction2 = promptForFraction("two"); // second fraction

    } catch (Exception e) {
        Messages.inputError(e); // display error
        return;
    }

    // snip...
}

Jamal · Accepted Answer · 2014-08-17 15:24:26Z

3

From a mathematical viewpoint it would be important that your fractions will be reduced to a minimal representation by removing common factors to numerator and denominator. So a private function Fraction.normalize is expected to reduce the terms and put the (possible) negative sign in the numerator.

This is useful for several reasons:

numerator and denominator could grow so big that computations are no longer possible;
it makes easier to compare two fractions
it makes output more clean: \$\frac{-1}{2}\$ is far better than \$\frac{5}{-10}\$

edited Aug 17, 2014 at 15:24

Jamal

34.8k13 gold badges132 silver badges237 bronze badges

answered Aug 17, 2014 at 14:57

Emanuele Paolini

2,1471 gold badge13 silver badges14 bronze badges

Add a comment |

Stack Exchange Network

Java Fraction Calculator

5 Answers 5

Polymorphism

Your `Messages` class

Other suggestions

Fraction representation

Operator representation

Error handling

Your Answer

Not the answer you're looking for? Browse other questions tagged
java
calculator
rational-numbers
or ask your own question.

Hot Network Questions

Java Fraction Calculator

5 Answers 5

Polymorphism

Your Messages class

Other suggestions

Fraction representation

Operator representation

Error handling

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged javacalculatorrational-numbers or ask your own question.

Related

Hot Network Questions

Your `Messages` class

Not the answer you're looking for? Browse other questions tagged
java
calculator
rational-numbers
or ask your own question.