HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Simple calculator in Java using Swing and AWT

Submitted by: @import:stackexchange-codereview··
0
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

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:

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