patternjavaModerate
Beginner GUI Calculator
Viewed 0 times
beginnerguicalculator
Problem
I'm very new to Java and programming and I've written a calculator to put all the theory I've learned so far into practice. I'm sure there is a lot of room for improvement since I really just started learning. Any big mistakes you more experienced programmers notice immediately?
I'm especially intrigued in how you can separate this class into several pieces so that the code is more OO. Also, what about my
I guess positioning all the buttons myself isn't good practice either and I should use some sort of layout like a
```
import java.awt.EventQueue;
import java.awt.Font;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.SwingConstants;
import javax.swing.border.EmptyBorder;
public class Calculator extends JFrame implements ActionListener
{
private static final long serialVersionUID = 1L;
private JPanel contentPane;
private JTextField textField = new JTextField();
private JButton btnOne = new JButton("1");
private JButton btnTwo = new JButton("2");
private JButton btnThree = new JButton("3");
private JButton btnPlus = new JButton("+");
private JButton btnFour = new JButton("4");
private JButton btnFive = new JButton("5");
private JButton btnSix = new JButton("6");
private JButton btnMinus = new JButton("-");
private JButton btnSeven = new JButton("7");
private JButton btnEight = new JButton("8");
private JButton btnNine = new JButton("9");
private JButton btnStar = new JButton("*");
private JButton btnPoint = new JButton(".");
private JButton btnZero = new JButton("0");
private JButton btnEquals = new JButton("=");
private JButton
I'm especially intrigued in how you can separate this class into several pieces so that the code is more OO. Also, what about my
actionPerformed method? It looks so long and clunky.I guess positioning all the buttons myself isn't good practice either and I should use some sort of layout like a
GridLayout, but I purposely chose not to since I'm not sure whether Swing is really worth spending time on (I hear JavaFx is becoming more important).```
import java.awt.EventQueue;
import java.awt.Font;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.SwingConstants;
import javax.swing.border.EmptyBorder;
public class Calculator extends JFrame implements ActionListener
{
private static final long serialVersionUID = 1L;
private JPanel contentPane;
private JTextField textField = new JTextField();
private JButton btnOne = new JButton("1");
private JButton btnTwo = new JButton("2");
private JButton btnThree = new JButton("3");
private JButton btnPlus = new JButton("+");
private JButton btnFour = new JButton("4");
private JButton btnFive = new JButton("5");
private JButton btnSix = new JButton("6");
private JButton btnMinus = new JButton("-");
private JButton btnSeven = new JButton("7");
private JButton btnEight = new JButton("8");
private JButton btnNine = new JButton("9");
private JButton btnStar = new JButton("*");
private JButton btnPoint = new JButton(".");
private JButton btnZero = new JButton("0");
private JButton btnEquals = new JButton("=");
private JButton
Solution
At a glance,
You should use an array for the buttons.
Then loop to instantiate. While we're looping, we can add some lambda expressions to define what we want the buttons to do.
You should definitely follow Phrancis' advice on conventions, it'll make your code easier to read.
You can simplify your main method quite a bit by using a lambda expression.
Points to improve:
-
You should add some consideration to dividing by 0.
-
You can use more than one operator on the field, and it just errors out, despite being a valid operation (e.g. 3 + 2 - 1). Either disable more than 1 operation at a time or process the previous numbers when a new operator is clicked.
These points can be resolved fairly simply, e.g. in your
Great job with this, it's nice that you're applying your knowledge into a project, it's the best way to learn. It's better still that you've posted it here for CodeReview.
You should use an array for the buttons.
JButton[] buttons = new JButton[10];Then loop to instantiate. While we're looping, we can add some lambda expressions to define what we want the buttons to do.
for (int i = 0; i textField.setText(val));
}You should definitely follow Phrancis' advice on conventions, it'll make your code easier to read.
You can simplify your main method quite a bit by using a lambda expression.
public static void main(String[] args) {
SwingUtilities.invokeLater(Calculator::new);
}Points to improve:
-
You should add some consideration to dividing by 0.
-
You can use more than one operator on the field, and it just errors out, despite being a valid operation (e.g. 3 + 2 - 1). Either disable more than 1 operation at a time or process the previous numbers when a new operator is clicked.
- Don't display a double value if the inputs are only integers. (e.g. 2 + 3 = 5.0)
- For floating point division/multiplication consider rounding the value otherwise you have endlessly repeating decimals.
These points can be resolved fairly simply, e.g. in your
divide methodif (lastCharInt != 0) {
double result = firstCharsInt / lastCharsInt;
} else {
// set a message that notes that you cannot divide by 0 and clear
}
String stringResult = result == (int)result ? Integer.toString((int)result) : String.format(%.2f, result);Great job with this, it's nice that you're applying your knowledge into a project, it's the best way to learn. It's better still that you've posted it here for CodeReview.
Code Snippets
for (int i = 0; i < buttons.length; i++) {
String val = Integer.toString(i)
buttons[i] = new JButton(val);
buttons[i].addActionListener(e -> textField.setText(val));
}public static void main(String[] args) {
SwingUtilities.invokeLater(Calculator::new);
}if (lastCharInt != 0) {
double result = firstCharsInt / lastCharsInt;
} else {
// set a message that notes that you cannot divide by 0 and clear
}
String stringResult = result == (int)result ? Integer.toString((int)result) : String.format(%.2f, result);Context
StackExchange Code Review Q#97394, answer score: 10
Revisions (0)
No revisions yet.