Layouts
I agree with @Simon's comment to use a layout manager, so that you don't need to use hard-coded numbers for positioning with the added benefit of freely resizing your application without worrying about alignment.
Dealing with repetition
Another way of getting the button text that triggered the ActionListener
is the following:
public void actionPerformed(ActionEvent event) {
JButton sourceButton = (JButton) event.getSource();
answerField.setText(answerField.getText() + sourceButton.getText());
}
Going this way means you don't really need to re-specify your buttons' text and the values you want to store differently. If you happen to be on Java 8, the functional interfaces feature mean that your ActionListener
need not be more than:
buttonX.addActionListener(event -> {
appendText(((JButton) event.getSource()).getText());
});
Where appendText()
is a simple wrapper method:
private void appendText(String value) {
answerField.setText(answerField.getText() + value);
}
The idea behind this being to reduce the amount of repetitive code.
Calculation (and validation)
I think it's fine to get a ScriptEngine
and then eval()
the mathematical expression, but this two-step process is not as instantaneous. Therefore, I will prefer to store engine
as a class-level field, and pressing the =
button will only perform the evaluation. You may want to do a dummy evaluation first so that the very first calculation performed appears faster.
As you should have assessed by now, providing a reduced set of mathematical operators and doing calculations are easy, but validating and accepting a simple mathematical expression... not so much. Shameless plug alert: my take on a GUI calculator attempts to resolve that by enabling or disabling buttons depending on the existing input, so you may want to consider something like that. In that case, you wouldn't have to be concerned about a user entering + - * / =
.
Should ScriptEngine
throw an Exception
related to your text field (e.g. when users absent-mindedly press the next expression to give you the following 'garbage' input: 1 + 2 = 34 + 5<= error, do not compute>
), it will be more helpful to display it within your GUI, instead of just exception.printStackTrace()
. This prepares you for dealing with Exception
s properly in the future, too.
Memorization
This is where you run into two usability-breaking problems:
- You memorize the entire text field instead of the previous result
- You don't have two separate buttons to separate the features of clearing the screen or the memory
Effectively, this means your memorization is broken as I can't reuse my past result! In order to enter a new mathematical expression to use the (R)ecall
feature, I have to press (C)lear
... which clears the memory too. Without fixing this (by implementing point 2), it's as good as not providing any memorization at all. :(
One small bit of code improvement suggestion, you don't need an else if (negation-of-if-boolean-condition)
following an if
statement. That if
statement you have for your (M)emory
button should be just:
if (isMemorized) {
...
} else {
...
}
- 19.4k
- 3
- 37
- 93