patternjavaMinor
Calculator with history using Java 8
Viewed 0 times
historywithjavausingcalculator
Problem
Yesterday, I saw this post from another user: Calculator with history
I started refactoring the code while typing an answer but I end up not posting as an answer because I not sure if my implementation was good enough because of the difficulty I found to separate the concerns in that example.
I created my own implementation and tried to use new Java 8 features as much I as could and clean code practices (although I don't have much experience with these two).
I create some unit tests to ensure that the program works as the original. I will not post all the tests here, but I put it in a git repository as well as the project files: tests, project
```
import java.util.Optional;
import java.util.Scanner;
public class Calculator {
public static final String EXIT_MESSAGE = "Okay! Nobody misses you" +
"\n But here's the calculations you've done so far";
private static final String WRONG_ACTION_MESSAGE = "Shit, wrong answer, you'll have to calculate " +
"with these numbers again and then you can do whatever you want";
private HistoryHolder historyHolder = new HistoryHolder();
private Solver solver;
private Scanner scanner;
private AskForAction askForAction;
private AskForNumber askForFirstNumber;
private AskForNumber askForSecondNumber;
private AskForOperation askForOperation;
private double firstNumber;
private double secondNumber;
public Calculator(Solver solver, Scanner scanner) {
this.scanner = scanner;
this.solver = solver;
initializeAskers();
}
private void initializeAskers() {
askForAction = new AskForAction(this.scanner);
askForFirstNumber = new AskForNumber("Input the 1st number", this.scanner);
askForSec
I started refactoring the code while typing an answer but I end up not posting as an answer because I not sure if my implementation was good enough because of the difficulty I found to separate the concerns in that example.
I created my own implementation and tried to use new Java 8 features as much I as could and clean code practices (although I don't have much experience with these two).
I create some unit tests to ensure that the program works as the original. I will not post all the tests here, but I put it in a git repository as well as the project files: tests, project
import java.util.Scanner;
public class RunProgram {
public static void main(String[] args) {
Calculator calculator = new Calculator(new Solver(), new Scanner(System.in));
calculator.run();
}
}```
import java.util.Optional;
import java.util.Scanner;
public class Calculator {
public static final String EXIT_MESSAGE = "Okay! Nobody misses you" +
"\n But here's the calculations you've done so far";
private static final String WRONG_ACTION_MESSAGE = "Shit, wrong answer, you'll have to calculate " +
"with these numbers again and then you can do whatever you want";
private HistoryHolder historyHolder = new HistoryHolder();
private Solver solver;
private Scanner scanner;
private AskForAction askForAction;
private AskForNumber askForFirstNumber;
private AskForNumber askForSecondNumber;
private AskForOperation askForOperation;
private double firstNumber;
private double secondNumber;
public Calculator(Solver solver, Scanner scanner) {
this.scanner = scanner;
this.solver = solver;
initializeAskers();
}
private void initializeAskers() {
askForAction = new AskForAction(this.scanner);
askForFirstNumber = new AskForNumber("Input the 1st number", this.scanner);
askForSec
Solution
General
I was struggling about making this code review as in the first place I saw "a lot" of classes and separations of concerns (so my guess). But I went deeper into the analysis. Finally I came to the conclusion that something is strange about it.
Do not get me wrong. You have basic modularization. But this has gone into only one dimension. You separated concerns vertically but not horizontally.
Action
The first thing which makes me think about is not beneficial polymorphism.
An indicator for unfavourable polymorphism is that no behaviour is encapsulated. The only method in your Action enum with something to do is "byActionNumber". And this method is not under polymorphism. "isStopCondition" and "isChangeNumbers" are unnecessary indirections. You can evaluate directly the type and it is worth the same. Maybe at least "isStopCondition" is adressing another aspect.
An inheritance hierarchy with each subclass providing same amount of boolean getters that will evaluate if the current instance is of a specific type makes this inheritance at least boiler-code. But furthermore you provide internal knowledge that you wanted to abstract in the first place.
Calculator
The Calculator has too many responsibilities even if it delegates tasks to its known "friends". Effectivly the Calculator "inherits" the responsibilities of all sub components.
So effectively your Calculator
... holds the process state
... gathers input from a scanner for process purposes
... produces intermediate output to the console for process purposes
... gathers input from a scanner for calculation purposes
... calculates (this is what a Calculator is all about)
... holds and maintains the history
... produces the calculation result as output
... produces the history as output
One reason for that: you haven't introduced abstractions at certain points. You are violating "dependency inversion principle".
You can easily figure out the resulting coupling and god-like responsibilities by asking following questions:
I provide an example that may be helpful to understand what I mean: See following code:
Now we remove the Calculator:
Now we remove the Input:
Now we remove the Console Output:
```
public static void main(String[] args) {
Input input = new Input();
Output output = new Output();
Calculator calculator = new Calculator();
calculator.setFirstOperand(input.getNumberFromKeyboard());
calculator.setOperator(inp
I was struggling about making this code review as in the first place I saw "a lot" of classes and separations of concerns (so my guess). But I went deeper into the analysis. Finally I came to the conclusion that something is strange about it.
Do not get me wrong. You have basic modularization. But this has gone into only one dimension. You separated concerns vertically but not horizontally.
Action
The first thing which makes me think about is not beneficial polymorphism.
An indicator for unfavourable polymorphism is that no behaviour is encapsulated. The only method in your Action enum with something to do is "byActionNumber". And this method is not under polymorphism. "isStopCondition" and "isChangeNumbers" are unnecessary indirections. You can evaluate directly the type and it is worth the same. Maybe at least "isStopCondition" is adressing another aspect.
An inheritance hierarchy with each subclass providing same amount of boolean getters that will evaluate if the current instance is of a specific type makes this inheritance at least boiler-code. But furthermore you provide internal knowledge that you wanted to abstract in the first place.
Calculator
The Calculator has too many responsibilities even if it delegates tasks to its known "friends". Effectivly the Calculator "inherits" the responsibilities of all sub components.
So effectively your Calculator
... holds the process state
... gathers input from a scanner for process purposes
... produces intermediate output to the console for process purposes
... gathers input from a scanner for calculation purposes
... calculates (this is what a Calculator is all about)
... holds and maintains the history
... produces the calculation result as output
... produces the history as output
One reason for that: you haven't introduced abstractions at certain points. You are violating "dependency inversion principle".
You can easily figure out the resulting coupling and god-like responsibilities by asking following questions:
- Can the input process work, even if a Calculator is not present? No.
- Can the Calculator work without the Scanner be present? No.
- Can the Calculator work without the Console output possibility to be present? No.
- Can the Calculator work without the History present? No.
I provide an example that may be helpful to understand what I mean: See following code:
public static void main(String[] args) {
Input input = new Input();
Output output = new Output();
Calculator calculator = new Calculator();
output.writeToConsole("Input the 1st number");
calculator.setFirstOperand(input.getNumberFromKeyboard());
output.writeToConsole("What to do?" +
"\n + for add" +
"\n - for minus" +
"\n * for multiply" +
"\n / for divide" +
"\n % for mod" +
"\n ^ for first number into the power of second number");
calculator.setOperator(input.getCharFromKeyboard());
output.writeToConsole("Input the 2nd number");
calculator.setSecondOperand(input.getNumberFromKeyboard());
calculator.execute();
output.writeToHistory(calculator.getCurrentResult());
output.writeToConsole(Double.toString(calculator.getCurrentResult()));
}Now we remove the Calculator:
public static void main(String[] args) {
Input input = new Input();
Output output = new Output();
output.writeToConsole("Input the 1st number");
input.getNumberFromKeyboard();
output.writeToConsole("What to do?" +
"\n + for add" +
"\n - for minus" +
"\n * for multiply" +
"\n / for divide" +
"\n % for mod" +
"\n ^ for first number into the power of second number");
input.getCharFromKeyboard();
output.writeToConsole("Input the 2nd number");
input.getNumberFromKeyboard();
}Now we remove the Input:
public static void main(String[] args) {
Output output = new Output();
Calculator calculator = new Calculator();
output.writeToConsole("Input the 1st number");
calculator.setFirstOperand(5);
output.writeToConsole("What to do?" +
"\n + for add" +
"\n - for minus" +
"\n * for multiply" +
"\n / for divide" +
"\n % for mod" +
"\n ^ for first number into the power of second number");
calculator.setOperator("+");
output.writeToConsole("Input the 2nd number");
calculator.setSecondOperand(7);
calculator.execute();
output.writeToHistory(calculator.getCurrentResult());
output.writeToConsole(Double.toString(calculator.getCurrentResult()));
}Now we remove the Console Output:
```
public static void main(String[] args) {
Input input = new Input();
Output output = new Output();
Calculator calculator = new Calculator();
calculator.setFirstOperand(input.getNumberFromKeyboard());
calculator.setOperator(inp
Code Snippets
public static void main(String[] args) {
Input input = new Input();
Output output = new Output();
Calculator calculator = new Calculator();
output.writeToConsole("Input the 1st number");
calculator.setFirstOperand(input.getNumberFromKeyboard());
output.writeToConsole("What to do?" +
"\n + for add" +
"\n - for minus" +
"\n * for multiply" +
"\n / for divide" +
"\n % for mod" +
"\n ^ for first number into the power of second number");
calculator.setOperator(input.getCharFromKeyboard());
output.writeToConsole("Input the 2nd number");
calculator.setSecondOperand(input.getNumberFromKeyboard());
calculator.execute();
output.writeToHistory(calculator.getCurrentResult());
output.writeToConsole(Double.toString(calculator.getCurrentResult()));
}public static void main(String[] args) {
Input input = new Input();
Output output = new Output();
output.writeToConsole("Input the 1st number");
input.getNumberFromKeyboard();
output.writeToConsole("What to do?" +
"\n + for add" +
"\n - for minus" +
"\n * for multiply" +
"\n / for divide" +
"\n % for mod" +
"\n ^ for first number into the power of second number");
input.getCharFromKeyboard();
output.writeToConsole("Input the 2nd number");
input.getNumberFromKeyboard();
}public static void main(String[] args) {
Output output = new Output();
Calculator calculator = new Calculator();
output.writeToConsole("Input the 1st number");
calculator.setFirstOperand(5);
output.writeToConsole("What to do?" +
"\n + for add" +
"\n - for minus" +
"\n * for multiply" +
"\n / for divide" +
"\n % for mod" +
"\n ^ for first number into the power of second number");
calculator.setOperator("+");
output.writeToConsole("Input the 2nd number");
calculator.setSecondOperand(7);
calculator.execute();
output.writeToHistory(calculator.getCurrentResult());
output.writeToConsole(Double.toString(calculator.getCurrentResult()));
}public static void main(String[] args) {
Input input = new Input();
Output output = new Output();
Calculator calculator = new Calculator();
calculator.setFirstOperand(input.getNumberFromKeyboard());
calculator.setOperator(input.getCharFromKeyboard());
calculator.setSecondOperand(input.getNumberFromKeyboard());
calculator.execute();
output.writeToHistory(calculator.getCurrentResult());
}public static void main(String[] args) {
Input input = new Input();
Output output = new Output();
Calculator calculator = new Calculator();
output.writeToConsole("Input the 1st number");
calculator.setFirstOperand(input.getNumberFromKeyboard());
output.writeToConsole("What to do?" +
"\n + for add" +
"\n - for minus" +
"\n * for multiply" +
"\n / for divide" +
"\n % for mod" +
"\n ^ for first number into the power of second number");
calculator.setOperator(input.getCharFromKeyboard());
output.writeToConsole("Input the 2nd number");
calculator.setSecondOperand(input.getNumberFromKeyboard());
calculator.execute();
// output.writeToHistory(calculator.getCurrentResult());
output.writeToConsole(Double.toString(calculator.getCurrentResult()));
}Context
StackExchange Code Review Q#155395, answer score: 2
Revisions (0)
No revisions yet.