First of all, you should read a bit on how to start a Swing application. You must take care of the Event Dispatcher Thread when starting your application. And, because of the way AWT manage threads, you don't need this infinite loop that will just take resources for nothing. The main thread will stay active until your frame is closed[1].
View
For the GUI, it is always a good idea to extract your components. You can create other classes, inner classes or factory methods to distinct your TextPanel, ButtonsPanel and the frame itself. Keep in mind the single responsibility principle.
Gui(Calculator calculator) {
this.calculator = calculator;
setSize(400, 400);
setDefaultCloseOperation(EXIT_ON_CLOSE);
add(text = new TextPanel(), BorderLayout.NORTH);
add(buttons = new ButtonsPanel(text), BorderLayout.SOUTH);
}
The ButtonsPanel is the most complex part of your view. The buttons loop and the event handling should be improved. Let's start with the deepest for. Did you know that you can set one actionCommand on a JButton this will be useful to assign one value to one button and drop the loop inside the event handling.
But there is better, you can create your own action or button to clearly expose this aspect. To improve your code readability you can also introduce some constants for the special buttons. By doing that you remove the comparison on array indexes and have a code that start to be auto documented
ButtonsPanel(final TextPanel screen, final Calculator calculator) {
super(new java.awt.GridLayout(4, 5));
setPreferredSize(new Dimension(380, 280));
for (int i = 0; i < buttonText.length; i++) {
add(new ActionButton(buttonText[i]));
}
}
private void onButtonPressed(String symbol) {
if (ERASE.equals(symbol)) {
String text = screen.getText();
text = text.substring(0, text.length() - 1);
screen.setText(text);
} else if (CLEAR.equals(symbol)) {
screen.setText("");
} else if (EQUAL.equals(symbol)) {
String text = screen.getText();
String output = "";
try {
output = calculator.calculate(text);
} catch (Exception ex) {
}
screen.setText(output);
} else {
String fieldText = screen.getText();
screen.setText(fieldText + symbol);
}
}
At this time you can notice that most of your logic is into this ButtonsPanel. This is not a good thing because testing it is not easy and this class mixes presentation and logic.
The de-facto pattern for Swing is MVC, you have already noticed that all existing components are using listeners. And you can do the same. Create one listener that will be used by your Gui to be notified when one button is pressed. In the meantime, if you are using Java 9 or bigger, you can use a switch expression to replace your ifs.
public Gui(final Calculator calculator) {
this.calculator = calculator;
setSize(400, 400);
setDefaultCloseOperation(EXIT_ON_CLOSE);
add(display = new TextPanel(), BorderLayout.NORTH);
add(buttons = new ButtonsPanel(display, calculator), BorderLayout.SOUTH);
buttons.addListener(symbol -> {
StringBuilder equation = new StringBuilder(display.getText());
switch (symbol) {
case ButtonsPanel.ERASE:
equation.deleteCharAt(equation.length()-1);
break;
case ButtonsPanel.CLEAR:
equation.delete(0, equation.length());
break;
case ButtonsPanel.EQUAL:
String output = "";
try {
output = calculator.calculate(equation.toString());
} catch (Exception ex) {
// FIXME
}
equation.replace(0, equation.length(), output);
break;
default:
equation.append(symbol);
}
display.setText(equation.toString());
});
}
While this is still better, in my opinion, you are still exposing too much details onf your ButtonsPanel (what if you want to use icons instead of String). The ButtonsListener should have dedicated method for the special symbols.
interface ButtonsPanelListener extends EventListener {
void onInput(String text);
void onClearScreen();
void onEraseOne();
void onEqual();
}
By doing that you will move the interpretation of the symbols back to the ButtonsPanel. Another improvement that you can do is create a class for all your buttons with a callback to trigger the correct method on the listener. By doing that the behavior is attached to the button and the reader will not have to scroll into the sources to find how each button behave.
interface Key {
String getText();
void onPressed(ButtonsPanelListener listener);
}
This will clean the "view" part. But, as already said, Swing is an MVC framework. So you are still missing the "m" and "c" parts...
Model
The model must be where your "state" is stored. Ideally this is the only class where you set and get the text. Let's create an Equation class for it and add all the required method on it (append, clear, dropLast, ..)
So the Gui change this model when he receive an event from his ButtonsPanel. And you ends-up a line that is duplicate for each kind of event:
display.setText(equation.getText());
In Swing, the model is usually observable, so you can also add some listeners to react when he change. With this observable model your Gui will receive events form the buttons and update the model. When updated, the model will notify the text (via the event listener) and the text will change.
+---------------+ +-----+ +-----------+ +-----------+
| ButtonsPanel | | Gui | | Equation | | TextPanel |
+---------------+ +-----+ +-----------+ +-----------+
| | | |
| onInput | | |
|------------>| | |
| | | |
| | append | |
| |------------->| |
| | | |
| | | onChange |
| | |--------------->|
| | | |
| | | | setText
| | | |--------
| | | | |
| | | |<-------
| | | |
You can notice that from one side, there is ane actor between the event and the model (the Gui between ButtonsPanel and Equation), while on the other side there is no actor between the model and one view (Equation to TextPanel). This incoherence lead us to a little architectural discussion.
It is up to you to decide who will listen and react to an event. But, in my case, I rely on the smart and dumb containers pattern. Where I have one smart component, the Gui and the two dumbs, ButtonsPanel and TextPanel. So that the Gui is a kind of mediator between the model and the views.
+-----------+ +-----+ +---------------+ +-----------+
| Equation | | Gui | | ButtonsPanel | | TextPanel |
+-----------+ +-----+ +---------------+ +-----------+
| | | |
| | onInput | |
| |<-----------------| |
| | | |
| append | | |
|<------------| | |
| | | |
| onChange | | |
|------------>| | |
| | | |
| | setText | |
| |--------------------------------->|
| | | |
In fact the Gui has too much responsibility, because it play both the role of a view (a smart container) and the role of a controller.
Controller
Let's segregate the roles of the Gui is role will be to deal with the presentation and redirect events to a controller. On the other side the controller will update the model according to the received actions.
In some systems the controller implements all the required listeners. This is sometime not the best one for code reuse because the controller depends on Swing. Also it require to expose the listeners of all of your components or create adapters between the listeners for the dumb components and those of the smart components. But it is useful to reduce the number of delegation or decoration methods. The biggest advantage is that it force you to create one model that represent the state of your application (or part of it) so that any change can be observed by the view.
Finally
A this time you should have a clean MVC. The advantages of this pattern is that you can easily test the model and the controller who are usually the most critical parts in an MVC. You can also extract your core business (representation and execution of an equation) to reusable and testable classes and build your model (and sometimes controller) over them by applying the decorator and adapter patterns.
Please note that there are other popular pattern to build one application, Model View Presenter is one of them.
+-------+ +-------+ +---------------+ +-----------+ +-------------+
| Model | | View | | ButtonsPanel | | TextPanel | | Controller |
+-------+ +-------+ +---------------+ +-----------+ +-------------+
| | | | |
| | onButtonPressed | | |
| |<-------------------------| | |
| | | | |
| | // call method for the pressed button | |
| |-------------------------------------------------------->|
| | | | |
| | | // call mutation method |
|<------------------------------------------------------------------------|
| | | | |
| onChange | | | |
|-------------->| | | |
| | | | |
| | setText | | |
| |----------------------------------------->| |
| | | | |
Model
class Model {
private final EventListenerList listeners = new EventListenerList();
private final StringBuilder content;
public Model() {
this.content = new StringBuilder();
}
public void setResult(String result) {
fireOnResult(result);
}
public void append(String part) {
content.append(part);
fireOnChange();
}
public void clear() {
content.delete(0, content.length());
fireOnChange();
}
public void dropLast() {
content.deleteCharAt(content.length());
fireOnChange();
}
public String getText() {
return content.toString();
}
public void addListener(ModelListener listener) {
listeners.add(ModelListener.class, listener);
}
public void removeListener(ModelListener listener) {
listeners.remove(ModelListener.class, listener);
}
private void fireOnChange() {
ModelListener[] lstnrs = listeners.getListeners(ModelListener.class);
for (int i=lstnrs.length-1; i > -1; i--) {
lstnrs[i].onEquationChange(content.toString());
}
}
private void fireOnResult(String result) {
ModelListener[] lstnrs = listeners.getListeners(ModelListener.class);
for (int i=lstnrs.length-1; i > -1; i--) {
lstnrs[i].onResult(result);
}
}
}
View
class View extends JFrame {
private final Controller controller;
private final ButtonsPanel buttons;
private final TextPanel display;
public View(final Controller controller) {
this.controller = controller;
setSize(400, 400);
setDefaultCloseOperation(EXIT_ON_CLOSE);
add(display = new TextPanel(), BorderLayout.NORTH);
add(buttons = new ButtonsPanel(), BorderLayout.SOUTH);
buttons.addListener(new ButtonsListener());
controller.addModelListener(new TextUpdater());
}
private final class TextUpdater implements ModelListener {
private void updateText(String newText) {
SwingUtilities.invokeLater(() -> display.setText(newText));
}
@Override
public void onEquationChange(String equation) {
updateText(equation);
}
@Override
public void onResult(String result) {
updateText(result);
}
}
private final class ButtonsListener implements ButtonsPanel.ButtonsPanelListener {
@Override
public void onButtonPressed(String text) {
new SwingWorker<Void, Void>(){
@Override
protected Void doInBackground() throws Exception {
switch (text) {
case ButtonsPanel.CLEAR:
controller.clear();
break;
case ButtonsPanel.ERASE:
controller.eraseOne();
break;
case ButtonsPanel.EQUAL:
controller.compute();
break;
default:
controller.onInput(text);
}
return null;
}
}.execute();
}
}
}
Controller
class Controller {
private final Engine engine;
private final Model model;
public Controller(Engine engine) {
this.engine = engine;
this.model = new Model();
}
public void compute() throws Exception {
String result = engine.compute(model.getText());
model.setResult(result);
}
public void onInput(String text) {
model.append(text);
}
public void clear() {
model.clear();
}
public void eraseOne() {
model.dropLast();
}
public void addModelListener(ModelListener listener) {
model.addListener(listener);
}
public void removeModelListener(ModelListener listener) {
model.removeListener(listener);
}
}
[1] : https://docs.oracle.com/javase/7/docs/api/java/awt/doc-files/AWTThreadIssues.html
The sequence diagrams were made with TextArt.io : https://textart.io/sequence
