3
\$\begingroup\$

I finished my first calculator using Java!

I try to apply Object-Oriented Programming!

I'll add my gitHub link if you're suited to viewing code on gitHub.

Here is a link to my GitHub Calculator Project

dvdev04-github-calculator

I enabled the calculator to calculate data type of Double, so it can get result containing decimals.

When reviewing, can you focus on whether my code

  • follows clean code
  • classes are divided reasonably
  • handled exceptions well (such when doing calculation)
  • has no redundant or repeated codes

Main Class

enter code hereimport java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class Main {
public static void main(String[] args) throws IOException {
    BufferedReader br = new BufferedReader(new InputStreamReader(System.in));

    // take in option chose in the menu & instantiate all the objects
    Menu menu = new Menu();
    Calculator calculate = new Calculator();
    Database database = new Database();
    EquationConverter equationConverter = new EquationConverter();
    EquationFormatChecker equationFormatChecker = new EquationFormatChecker();


    // Starting Calculator
    System.out.println("Calculator Initializing...");

    // Main Loop of Options
    while(true) {
        // Checking Input if it's 1 or 2 or 3
        while(true) {
            System.out.println("-------------------");
            System.out.println("1. History\n2. Calculation\n3. Terminate");
            try {
                menu.setMenuChoice(Integer.parseInt(br.readLine()));
                if (menu.getMenuChoice() >= 1 && menu.getMenuChoice() <= 3) {
                    break;
                }
            } catch (NumberFormatException e) {
                System.out.println(">>> Wrong Input, please try again.");
            }
        }

        // using if else to figure out whether to view history/perform calculation/exit program
        switch (menu.getMenuChoice()) {
            // view history
            case 1 -> {
                if (database.checkEmpty()) {
                    System.out.println("Retrieving History...\n");
                    System.out.println(database.getHistory());
                } else {
                    System.out.println(">>> History's Empty");
                }
            }
            // do calculation
            case 2 -> {
                boolean continueCalculation = true;
                while (continueCalculation) {
                    try {
                        System.out.println("-------------------");
                        System.out.print("Enter equation (Example: 1 + 4 * 5 / 2)\n=> ");
                        // take in the equation as String and pass it on to equation convertor
                        String equation = br.readLine();
                        equationConverter.setEverything(equation);
                        // checking validity of equation
                        boolean validityCheck = equationFormatChecker.checkValidity(equationConverter.getNumbersList(), equationConverter.getOperatorsList());
                        if (validityCheck) {
                            // code is in this order because of possible ArithmeticException in calculate

                            // find the answer
                            Double answer = calculate.calculateEquation(equationConverter.getNumbersList(), equationConverter.getOperatorsList());

                            // add equation to history (only equation, not answer)
                            database.addHistory(equation);
                            System.out.println("Answer: " + answer);
                            //and add answer to the database
                            database.addAnswerToEquation(answer);
                            continueCalculation = false;
                        }
                    } catch (NumberFormatException e1) {
                        System.out.println("Wrong format, try again.");
                    } catch (ArithmeticException e2) {
                        if (e2.getMessage().equals("Infinite"))
                            System.out.println("Can't divide by zero. Try again.");
                        if (e2.getMessage().equals("NaN")) System.out.println("Not a Number. Try Again");
                    }
                }
            }

            // exit program
            case 3 -> {
                System.out.println("Terminating calculator...");
                System.exit(0);
            }
        }
    }
}
}

Menu Class

public class Menu {
private int menuChoice;

public void setMenuChoice(int menuChoice) {
    this.menuChoice = menuChoice;
}

public int getMenuChoice() {
    return menuChoice;
}
}

EquationConverter Class

import java.util.ArrayList;
import java.util.List;

public class EquationConverter {
private ArrayList<String> equationList;
private ArrayList<Double> numbersList;
private ArrayList<String> operatorsList;



// initialize equationList
EquationConverter() {
    equationList = new ArrayList<>();
    numbersList = new ArrayList<>();
    operatorsList = new ArrayList<>();
}

// set the Equation
public void setEverything(String equationString) {
    equationList.clear();
    equationList.addAll(List.of(equationString.split(" ")));
    setNumbersList();
    setOperatorsList();
}

// get equationList
public ArrayList<String> getEquationList() {
    return equationList;
}


// set list of numbers
public void setNumbersList() {
    numbersList = new ArrayList<>();
    for (int i = 0; i < equationList.size(); i += 2) {
        numbersList.add(Double.parseDouble(equationList.get(i)));
    }
}

// set list of operators
public void setOperatorsList() {
    operatorsList = new ArrayList<>();
    for (int i = 1; i < equationList.size(); i += 2) {
        operatorsList.add(equationList.get(i));
    }
}

public ArrayList<Double> getNumbersList() {
    return numbersList;
}

public ArrayList<String> getOperatorsList() {
    return operatorsList;
}
}

EquationFormatChecker Class

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class EquationFormatChecker {
private final List<String> operatorSet = new ArrayList<>(Arrays.asList("+", "-", "*", "/"));

// 0 for valid equation, 1 for invalid equation, 2 for division by zero
public boolean checkValidity(ArrayList<Double> numbers, ArrayList<String> operators) {
    int length = numbers.size() + operators.size();

    // checking if either List is empty or length is valid
    if (numbers.isEmpty() || operators.isEmpty() || length < 3 || length % 2 == 0) {
        return false;
    }

    // check if operatorsList contains + - * /
    for (String operator : operators) {
        if (!operatorSet.contains(operator)) {
            return false;
        }
    }
    return true;
}
}

Calculator Class

import java.util.ArrayList;

public class Calculator {
private ArrayList<Double> numberList;
private ArrayList<String> operatorList;


public double calculateEquation(ArrayList<Double> numbersList, ArrayList<String> operatorList) throws ArithmeticException {

    this.numberList = numbersList;
    this.operatorList = operatorList;

    for (int i = 0; i < operatorList.size(); i++) {
        if (operatorList.get(i).equals("*")) {
            numbersList.set(i + 1, numbersList.get(i) * numbersList.get(i + 1));
            operatorList.set(i, "+");
            numbersList.set(i, 0D);
        }
        if (operatorList.get(i).equals("/")) {
            double result = numbersList.get(i) / numbersList.get(i + 1);
            if (Double.isInfinite(result)) throw new ArithmeticException("Infinite");
            if (Double.isNaN(result)) throw new ArithmeticException("NaN");
            numbersList.set(i + 1, numbersList.get(i) / numbersList.get(i + 1));
            operatorList.set(i, "+");
            numbersList.set(i, 0D);
        }
    }

    Double accumulator = numbersList.get(0);

    for (int i = 1; i < numberList.size(); i++) {
        if (operatorList.get(i - 1).equals("+")) {
            accumulator += numbersList.get(i);
        }
        if (operatorList.get(i - 1).equals("-")) {
            accumulator -= numbersList.get(i);
        }
    }
    return accumulator;
}

public ArrayList<Double> getNumberList() {
    return numberList;
}

public ArrayList<String> getOperatorList() {
    return operatorList;
}
}

Database Class

import javax.xml.crypto.Data;
import java.util.ArrayList;

public class Database {
private ArrayList<String> historyList;
private String historyString;

Database() {
    this.historyList = new ArrayList<>();
}

public boolean checkEmpty() {
    if (historyList.isEmpty()) {
        return false;
    } return true;
}


// add a line of equation to the String
public void addHistory(String equation) {
    this.historyList.add(equation);
}

// return historyString
public String getHistory() {
    ArrayListToString();
    return historyString;
}

// convert historyList to String
private void ArrayListToString() {
    StringBuilder sb = new StringBuilder();
    for (String oneEquation : historyList) {
        sb.append(oneEquation.concat("\n"));
    }
    historyString = sb.toString();
}

// add answer to complete your equation
public void addAnswerToEquation(Double answer) {
    String stringAnswer = String.valueOf(answer);
    historyList.set(historyList.size() - 1, historyList.get(historyList.size() - 1).concat(" = " + stringAnswer));
}

public ArrayList<String> getHistoryList() {
    return historyList;
}

public String getHistoryString() {
    return historyString;
}
}
\$\endgroup\$
1

1 Answer 1

3
\$\begingroup\$

For a beginner this isn't terrible, but unfortunatly from a OOP point of view it's structured quite badly.


First i'm going to concentrate on the Database class. (Aside: a better name would be History or HistoryDatabase. To be honest most class and method names are badly chosen and inconsistant, but I'll ignore that for now).

One big point of OOP is to provide a self-contained object with a clear and sensible way to use it. Image that you wrote the class, but someone else will use it in their programm. It should be impossible for the user of the class to "break" it (or your class to break their programm). The class should preferably not allow to do "wrong" things at compile time, or at the very least notify the user (for example, through exceptions or some other error message/state) that what they are trying is wrong.

For example:

Database history = new Database();
// If the user would do the following, is it clear from what happens, what the problem is?
// history.addAnswerToEquation(3.0); 

history.addHistory("1 + 2");

// Does this do what you or the user may expect?
System.out.println(history.getHistoryString());

history.addAnswerToEquation(3.0);
history.addAnswerToEquation(3.0);
// Is the history object now in a "good" state?
System.out.println(history.getHistory());

So the main problem here is addAnswerToEquation. It should at the very least the method should make sure the class is in a state to accept an answer. For example:

public void addAnswerToEquation(Double answer) {
    if (historyList.isEmpty()) {
        throw IllegalStateException("At least one expression needs to be added before adding an answer.");
    }
    
    String lastEntry = historyList.get(historyList.size() - 1);
    if (lastEntry.contains("=")) {
        throw IllegalStateException("The last expression allready has an answer added.");
    }

    String stringAnswer = String.valueOf(answer);
    historyList.set(historyList.size() - 1, lastEntry.concat(" = " + stringAnswer));
}

However, is it really necessary to add the equation and the answer separately? It would probably be better to drop addAnswerToEquation and instead have addHistory take both the equation and the answer.


The three classes EquationConverter, EquationFormatChecker and Calculator have basically the same problem. Additionally the main functionality of the programm ist just arbitrary distributed over these three classes without any concrete connection between them. I don't have the time to go into details, but I'd suggest to first integrate the functionality of all three into a single Equation class. If more separation is needed, then this can be considered later.

\$\endgroup\$
2
  • \$\begingroup\$ So to see if I understand you correctly, minimize the redundant codes that will eventually do the same thing? (for adding and printing the equation and answer, which could've been done through single adding and single printing) Also that classes EquationConverter, EquationFormatChecker, Calculator just boils down to executing calculation so I should just make them into one class? \$\endgroup\$
    – dddleo
    Mar 23, 2023 at 2:00
  • \$\begingroup\$ I strongly suggest that you take a look at my comment in the post itself. By performing the expression evaluation in a well known manner you relieve yourself from having to implement coverters, format checkers and even the Equation class. You won't need to make tricks replacing multiplications and divisions with "+ 0" in order to maintain operator precedence. Reinventing the wheel can be fun and educational, but it's still better to reinvent a round wheel instead a square one. :) \$\endgroup\$ Mar 23, 2023 at 7:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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