patternjavaMinor
Simple calculator in Java using Swing and AWT
Viewed 0 times
simpleawtjavaswingusingandcalculator
Problem
I wrote a simple calculator with general operations. Give me please some advice, suggestions, and criticism about my code: code design, readability, mistakes.
Calculator.java
CalculatorEngine.java
```
public class CalculatorEngine {
private enum Operator {
ADD, SUBTRACT, MULTIPLY, DIVIDE
}
private double currentTotal;
public String getTotalString() {
return currentTotal % 1.0 == 0
? Integer.toString((int) currentTotal)
: String.valueOf(currentTotal);
}
public void equal(String number) {
currentTotal = Double.parseDouble(number);
}
public void add(String number) {
convertToDouble(number, Operator.ADD);
}
public void subtract(String number) {
convertToDouble(number, Operator.SUBTRACT);
}
public void multiply(String number) {
convertToDouble(number, Operator.MULTIPLY);
}
public void divide(String number) {
convertToDouble(number, Operator.DIVIDE);
}
private void convertToDouble(String number, Operator operator) {
double dblNumber = Double.parseDouble(number);
switch (operator) {
case ADD:
add(dblNumber);
break;
case SUBTRACT:
subtract(dblNumber);
break;
case MULTIPLY:
multiply(dblNumber);
break;
case DIVIDE:
divide(dblNumber);
break;
default:
throw new AssertionError(operator.name());
}
}
private void add(double number) {
currentTotal += number % 1.0 == 0 ? (int) nu
Calculator.java
import javax.swing.*;
public class Calculator {
public static void main(String[] args) {
CalculatorView calculator = new CalculatorView();
// Windows settings
calculator.setTitle("Simple Calculator");
calculator.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
}
}CalculatorEngine.java
```
public class CalculatorEngine {
private enum Operator {
ADD, SUBTRACT, MULTIPLY, DIVIDE
}
private double currentTotal;
public String getTotalString() {
return currentTotal % 1.0 == 0
? Integer.toString((int) currentTotal)
: String.valueOf(currentTotal);
}
public void equal(String number) {
currentTotal = Double.parseDouble(number);
}
public void add(String number) {
convertToDouble(number, Operator.ADD);
}
public void subtract(String number) {
convertToDouble(number, Operator.SUBTRACT);
}
public void multiply(String number) {
convertToDouble(number, Operator.MULTIPLY);
}
public void divide(String number) {
convertToDouble(number, Operator.DIVIDE);
}
private void convertToDouble(String number, Operator operator) {
double dblNumber = Double.parseDouble(number);
switch (operator) {
case ADD:
add(dblNumber);
break;
case SUBTRACT:
subtract(dblNumber);
break;
case MULTIPLY:
multiply(dblNumber);
break;
case DIVIDE:
divide(dblNumber);
break;
default:
throw new AssertionError(operator.name());
}
}
private void add(double number) {
currentTotal += number % 1.0 == 0 ? (int) nu
Solution
Usability issues
Some things don't work as I would expect:
It would be good to optimize make the user interface a bit friendlier.
Separation of concerns
It's good that you separated the engine, the view, and the main class that just sets up and runs everything.
But it would be good to go further.
The calculations are performed by the engine,
and controlled by an action listener implemented inside the view,
using a switch.
Instead of a switch,
it would be better to abstract the calculation logic,
for example using an
The
In that setup,
it will just know that each operation implements
and has an
That will be more flexible and extensible.
Naming
Many of the method and variable names are quite good,
but there are some bad ones that stand out, for example in this code:
There is
but they are quite different in nature.
The first contains operators used in calculations,
the second is more about controlling the application,
which is different from performing calculations.
So instead of numbering variables,
you could give them more meaningful names.
Some things don't work as I would expect:
- Pressing the equals button twice in a row or after clearing gives "ERROR - wrong operation"
- Pressing an operation after the equal buttons (to continue calculations), gives "ERROR - wrong operation"
It would be good to optimize make the user interface a bit friendlier.
Separation of concerns
It's good that you separated the engine, the view, and the main class that just sets up and runs everything.
But it would be good to go further.
The calculations are performed by the engine,
and controlled by an action listener implemented inside the view,
using a switch.
Instead of a switch,
it would be better to abstract the calculation logic,
for example using an
Operator interface with an apply method.The
Calculator class could configure CalculatorView with an arbitrary collection of Operator implementations.In that setup,
CalculatorView will not be aware of any of the calculation logic,it will just know that each operation implements
Operator,and has an
apply method to perform some calculation.That will be more flexible and extensible.
Naming
Many of the method and variable names are quite good,
but there are some bad ones that stand out, for example in this code:
for (String anOperationPanelNames1 : operationPanelNames1) {
JButton b = new JButton(anOperationPanelNames1);
operationPanel1.add(b);
b.addActionListener(operationListener);
}anOperationPanelNames1 is the most terrible name in the code.b is not great either, spelling out to button would make it a tad more readable, and not terribly long.There is
operationPanel1 and operationPanel2,but they are quite different in nature.
The first contains operators used in calculations,
the second is more about controlling the application,
which is different from performing calculations.
So instead of numbering variables,
you could give them more meaningful names.
Code Snippets
for (String anOperationPanelNames1 : operationPanelNames1) {
JButton b = new JButton(anOperationPanelNames1);
operationPanel1.add(b);
b.addActionListener(operationListener);
}Context
StackExchange Code Review Q#92643, answer score: 4
Revisions (0)
No revisions yet.