patternjavaMinor
Java GUI 2-input Calculator
Viewed 0 times
inputguicalculatorjava
Problem
I have written this code for a two input calculator. Is there anything I can do to make this code more efficient?
```
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
public class CalcGui extends JFrame {
private JButton buttonZero;
private JButton buttonOne;
private JButton buttonTwo;
private JButton buttonThree;
private JButton buttonFour;
private JButton buttonFive;
private JButton buttonSix;
private JButton buttonSeven;
private JButton buttonEight;
private JButton buttonNine;
private JButton opButtonPlus;
private JButton opButtonMinus;
private JButton opButtonDivide;
private JButton opButtonMultiply;
private JButton opButtonEquals;
private JButton opButtonClear;
private TextField tf;
private JPanel numButtonPanel;
private JPanel opButtonPanel;
private JPanel basePanel;
public CalcGui(){
super("Scientific Calculator");
basePanel = new JPanel();
numButtonPanel = new JPanel(new GridLayout(0,4));
opButtonPanel = new JPanel(new GridLayout(0,1));
tf = new TextField(20);
tf.setEditable(false);
basePanel.add(tf);
buttonZero = new JButton("0");
numButtonPanel.add(buttonZero);
buttonOne = new JButton("1");
numButtonPanel.add(buttonOne);
buttonTwo = new JButton("2");
numButtonPanel.add(buttonTwo);
buttonThree = new JButton("3");
numButtonPanel.add(buttonThree);
buttonFour = new JButton("4");
numButtonPanel.add(buttonFour);
buttonFive = new JButton("5");
numButtonPanel.add(buttonFive);
buttonSix = new JButton("6");
numButtonPanel.add(buttonSix);
buttonSeven = new JButton("7");
numButtonPanel.add(buttonSeven);
buttonEight = new JButton("8");
numButtonPanel.add(buttonEight);
buttonNine = new JButton("9");
numButtonPanel.add(buttonNine)
```
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
public class CalcGui extends JFrame {
private JButton buttonZero;
private JButton buttonOne;
private JButton buttonTwo;
private JButton buttonThree;
private JButton buttonFour;
private JButton buttonFive;
private JButton buttonSix;
private JButton buttonSeven;
private JButton buttonEight;
private JButton buttonNine;
private JButton opButtonPlus;
private JButton opButtonMinus;
private JButton opButtonDivide;
private JButton opButtonMultiply;
private JButton opButtonEquals;
private JButton opButtonClear;
private TextField tf;
private JPanel numButtonPanel;
private JPanel opButtonPanel;
private JPanel basePanel;
public CalcGui(){
super("Scientific Calculator");
basePanel = new JPanel();
numButtonPanel = new JPanel(new GridLayout(0,4));
opButtonPanel = new JPanel(new GridLayout(0,1));
tf = new TextField(20);
tf.setEditable(false);
basePanel.add(tf);
buttonZero = new JButton("0");
numButtonPanel.add(buttonZero);
buttonOne = new JButton("1");
numButtonPanel.add(buttonOne);
buttonTwo = new JButton("2");
numButtonPanel.add(buttonTwo);
buttonThree = new JButton("3");
numButtonPanel.add(buttonThree);
buttonFour = new JButton("4");
numButtonPanel.add(buttonFour);
buttonFive = new JButton("5");
numButtonPanel.add(buttonFive);
buttonSix = new JButton("6");
numButtonPanel.add(buttonSix);
buttonSeven = new JButton("7");
numButtonPanel.add(buttonSeven);
buttonEight = new JButton("8");
numButtonPanel.add(buttonEight);
buttonNine = new JButton("9");
numButtonPanel.add(buttonNine)
Solution
The first (and biggest) recommendation I have is to use a
Secondly, I would recommend if you consistently name your components. I'm not sure what
If you notice in the above code, I've also added
Now, inside your constructor, I'd recommend getting rid of some magic numbers:
Next, as you are creating your new buttons, I would recommend creating a different
with the function:
Then when you create your operator buttons, you can add the appropriate logic right there, instead of doing it all in the same function.
JButton array to store your numeric buttons. This will drastically reduce the amount of duplicate code you have written:private final List numberButtons;
private final static int NUMBER_OF_DIGITS = 10;Secondly, I would recommend if you consistently name your components. I'm not sure what
op stands for, but you can either list the component type first, or list it last, but do it consistently. I've decided to put the component type after:private final JButton plusButton;
private final JButton minusButton;
private final JButton divideButton;
private final JButton multiplyButton;
private final JButton equalsButton;
private final JButton clearButton;
private final TextField displayField;
private final JPanel numbersPanel;
private final JPanel operatorsPanel;
private final JPanel basePanel;If you notice in the above code, I've also added
final to each of the fields, as you don't plan on changing them.Now, inside your constructor, I'd recommend getting rid of some magic numbers:
private static final String TITLE = "Scientific Calculator";
private static final int NUMBERS_PER_ROW = 4;
private static final int OPERATORS_PER_ROW = 1;
private static final int OUTPUT_FIELD_WIDTH = 20;Next, as you are creating your new buttons, I would recommend creating a different
ActionListener for each button. This will remove the massive if/else you have in your HandlerClass. Also, you are frequently appending to the text, so using a function for that would be ideal. For example, for (int i = 0; i < NUMBER_OF_DIGITS; i++){
final JButton button = new JButton(""+i);
button.addActionListener(new ActionListener(){
public void actionPerformed(ActionEvent e){
addToDisplay(button.getText());
}
});
}with the function:
private void addToDisplay(String text){
displayField.setText(displayField.getText()+text);
}Then when you create your operator buttons, you can add the appropriate logic right there, instead of doing it all in the same function.
Code Snippets
private final List<JButton> numberButtons;
private final static int NUMBER_OF_DIGITS = 10;private final JButton plusButton;
private final JButton minusButton;
private final JButton divideButton;
private final JButton multiplyButton;
private final JButton equalsButton;
private final JButton clearButton;
private final TextField displayField;
private final JPanel numbersPanel;
private final JPanel operatorsPanel;
private final JPanel basePanel;private static final String TITLE = "Scientific Calculator";
private static final int NUMBERS_PER_ROW = 4;
private static final int OPERATORS_PER_ROW = 1;
private static final int OUTPUT_FIELD_WIDTH = 20;for (int i = 0; i < NUMBER_OF_DIGITS; i++){
final JButton button = new JButton(""+i);
button.addActionListener(new ActionListener(){
public void actionPerformed(ActionEvent e){
addToDisplay(button.getText());
}
});
}private void addToDisplay(String text){
displayField.setText(displayField.getText()+text);
}Context
StackExchange Code Review Q#97005, answer score: 7
Revisions (0)
No revisions yet.