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

Basic Calculator in Java with Swing

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withjavaswingcalculatorbasic

Problem

I used a standard calculator design from Java. I wanted to expand it so I created a class to create buttons for different operations, like +, -, *, /. The original program didn't do this, they just made them individually without a template method. Do you think I should do it like this?

```
//The java Template Calculator TODO

import java.awt.EventQueue;
import java.awt.GridLayout;
import java.awt.BorderLayout;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.JButton;
import java.awt.Container;

public class JavaCalculator implements ActionListener{

JFrame guiFrame;
JPanel buttonPanel;
JTextField numberCalc;
int calcOperation = 0;
int currentCalc;

public static void main(String[] args) {

EventQueue.invokeLater(new Runnable()
{

public void run()
{

new JavaCalculator();
}
});

}

public JavaCalculator()
{
guiFrame = new JFrame();
guiFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
guiFrame.setTitle("Simple Calculator");
guiFrame.setSize(300,300);
guiFrame.setLocationRelativeTo(null);
numberCalc = new JTextField();
numberCalc.setHorizontalAlignment(JTextField.RIGHT);
numberCalc.setEditable(false);
guiFrame.add(numberCalc, BorderLayout.NORTH);
buttonPanel = new JPanel();
buttonPanel.setLayout(new GridLayout(4,4));
guiFrame.add(buttonPanel, BorderLayout.CENTER);

for (int i=0;i<10;i++)
{
addNumberButton(buttonPanel, String.valueOf(i));
}

addActionButton(buttonPanel, 1, "+");
addActionButton(buttonPanel, 2, "-");
addActionButton(buttonPanel, 3, "*");
addActionButton(buttonPanel, 4, "/");
addActionButton(buttonPanel, 5, "^2");

JButton equalsButton = new JButton("=");
equalsButton.setActionCommand("=");
equalsButton.addActionListener(new ActionListener()
{

Solution

Bug!

In your program, it's impossible to use any operator besides addition. As Code Review is not meant to be a code-bug-fixing site, I think you should try to fix this yourself. Hint: The bug is in your addActionButton method
Now, about your question

Yes, the template method is a good choice. Removing code duplication is always a good idea, which is I have to inform you about some other improvements you can make as well.
Some things you should think about first

  • Java coding convention is to put the { on the same line as before, not on it's own line.



  • It is good practice to use private final on as many class-variables ("fields") as possible.



  • It is an even better practice to reduce the scope of variables when possible, buttonPanel and guiFrame can be declared as local variables in the constructor.



Reducing duplication

  • numberCalc.setText(Integer.toString(calculate)); is done on every if-case, put it after all the if-else statements instead.



  • Instead of using several if-else, you can use a switch statement.



  • An enum is a better choice than using an int to store the current calcOperation.



Java 8

If you are not allowed or unable to use Java 8, skip this section :)

If you can use Java 8, there's a way to do this very smoothly. It may be a bit advanced if you are new to the Java language, but if you are willing to learn I suggest you check out IntBinaryOperator.

Your int calcOperation can be replaced with IntBinaryOperator operator;. Operations can be declared like this:

IntBinaryOperator add = (a, b) -> a + b;
IntBinaryOperator substract = (a, b) -> a - b;
IntBinaryOperator multiply = (a, b) -> a * b;
IntBinaryOperator divide = (a, b) -> a / b;


Your add action button can then become private void addActionButton(Container parent, IntBinaryOperator action, String text) {. This would greatly reduce code duplication for you, but as I said. This is only if you're using Java 8. If you're interested, add a comment and I'll explain more.

Regarding the user friendliness of your program, it's not layouted as a typical calculator. This is just a nitpick really. It would also be nice to use the keyboard for the program, but that's just a feature-request :)

Code Snippets

IntBinaryOperator add = (a, b) -> a + b;
IntBinaryOperator substract = (a, b) -> a - b;
IntBinaryOperator multiply = (a, b) -> a * b;
IntBinaryOperator divide = (a, b) -> a / b;

Context

StackExchange Code Review Q#45864, answer score: 11

Revisions (0)

No revisions yet.