Explain the working of the Java calculator gui.
Code Review
Java Calculator with GUI
Asked 2 years, 2 months ago
Modified 2 years, 2 months ago
Viewed 2k time
1
I've written a calculator in Java with a GUI using swing. Here are the classes:
Control.java:
public class Control {
public static void main(String[] args) {
Calculator calculator = new Calculator();
Gui gui = new Gui(calculator);
gui.setVisible(true);
boolean exit = false;
while(!exit) {
}
}
}
Gui.java:
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.util.logging.Level;
import java.util.logging.Logger;
public class Gui extends JFrame {
Calculator calculator;
public Gui(Calculator calculator) {
this.calculator = calculator;
//The frame
setSize(400, 400);
setDefaultCloseOperation(EXIT_ON_CLOSE);
//Panel for Text-output
JPanel textPanel = new JPanel();
textPanel.setPreferredSize(new Dimension(400,70));
JTextField textfield = new JTextField("");
textfield.setPreferredSize(new Dimension(380,70));
textfield.setHorizontalAlignment(JTextField.RIGHT);
textPanel.add(textfield);
add(textPanel, BorderLayout.NORTH);
//Panel for buttons
JPanel buttonPanel = new JPanel();
buttonPanel.setLayout(new java.awt.GridLayout(4, 5));
buttonPanel.setPreferredSize(new Dimension(380, 280));
//Buttons
String[] buttonText = {
"7", "8", "9", "+", "<-", "4", "5", "6", "-", "(", "1", "2", "3",
"*", ")", "0", ",", "C", "/", "="
};
JButton button[] = new JButton[20];
for(int i = 0; i < 20> button[i] = new JButton(buttonText[i]);
button[i].addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent event) {
for(int i = 0; i < 20>
if(event.getSource() == button[i]) {
//Delete last character button
if(i == 4) {
String text = textfield.getText().toString();
text = text.substring(0, text.length()-1);
textfield.setText(text);
}
//Delete all button
else if(i == 17) {
textfield.setText("");
}
//"="-button
else if(i == 19) {
String text = textfield.getText().toString();
String output = "";
try {
output = calculator.calculate(text);
}
catch (Exception ex) {
}
textfield.setText(output);
}
//other buttons
else {
String text = button[i].getText().toString();
String fieldText = textfield.getText().toString();
textfield.setText(fieldText + text);
}
}
}
}
});
buttonPanel.add(button[i]);
}
add(buttonPanel, BorderLayout.SOUTH);
}
}
Calculator.java:
import javax.script.ScriptEngine;
import javax.script.ScriptEngineManager;
public class Calculator {
public String calculate(String text) throws Exception {
ScriptEngineManager scriptEngineManager = new ScriptEngineManager();
ScriptEngine scriptEngine = scriptEngineManager.getEngineByName("JavaScript");
Object sol = scriptEngine.eval(text);
String solution = sol + "";
return solution;
}
}
What do you think about it? Do you have any suggestions on improving the code?
Control.java I don't like this name as it's not specific. I'd suggest 'CalculatorApp' instead for the Java calculator gui. You don't need the infinite loop. The application will already continue until the user closes the program.
// you can remove all of this:
boolean exit = false;
while(!exit) {
}Calculator.java
If you didn't write the program, would you know what Calculator.calculate(String) does? The naming is very ambiguous and the code itself looks very complicated (thankfully the heavy complicated, heavy lifting is done by an external library). You should add a javadoc to both the class & method at the very least. You could have done this entire assignment without JavaScript. You may have gotten more learning out of it if you didn't use ScriptEngine. To me it seems hacky. I do give you bonus points for cleverness, though.
Gui.java
You have 2 unused imports (Level & Logger)Avoid using wildcards (.*) in your imports. It clutters the local namespace and It makes it harder to tell which libraries are doing what. It will also cause a compiler error when there are two classes with the same name.
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;If you're using an IDE you should see a warning about your class not having a serialversionUID. You can allow your IDE to auto-generate one for you:
private static final long serialVersionUID = 1L;Avoid magic numbers & magic strings Your class would be easier to maintain & read if you used static variables declared at the top. E.G:
private static final int WIDTH = 400;
private static final int HEIGHT = 400;
...
//The frame
setSize(WIDTH, HEIGHT);No need to count the number of values in an array yourself, instead use array.length. This will make maintenance easier as you don't have to re-count the values every time:
JButton button[] = new JButton[buttonText.length];
for(int i = 0; i < buttonText xss=removed>