patternjavaMinor
Simple JavaFX Calculator
Viewed 0 times
simplecalculatorjavafx
Problem
I made a simple JavaFX calculator. It does basic calculations, and works to the best of my knowledge. However, I'm a novice at both Java and JavaFX, so I seriously doubt this is as efficient and clean as possible. I personally think it could be more modular, maybe put those huge loops in functions or something.
```
package calculator;
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.Group;
import javafx.scene.control.Label;
import javafx.scene.control.Button;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.GridPane;
import javafx.scene.layout.VBox;
import javafx.scene.layout.RowConstraints;
import javafx.scene.layout.ColumnConstraints;
import javafx.scene.paint.Color;
import javafx.event.ActionEvent;
import javafx.event.EventHandler;
import javafx.stage.Stage;
import javafx.geometry.HPos;
import java.util.ArrayList;
import javax.script.ScriptEngineManager;
import javax.script.ScriptEngine;
import javax.script.ScriptException;
public class Calculator extends Application{
private boolean operatorAlreadyPressed = false;
private boolean secondOperand = false;
private boolean scriptExceptionOccurred = false;
public static void main(String[] args){
Application.launch(args);
}
public void start(Stage primaryStage) throws ScriptException{
int windowWidth = 190;
int windowHeight = 250;
primaryStage.setTitle("Calculator");
primaryStage.setWidth(windowWidth);
primaryStage.setHeight(windowHeight);
// primaryStage.setResizable(false);
VBox root = new VBox();
Scene scene = new Scene(root, windowWidth, windowHeight, Color.WHITE);
Label expressionLabel = new Label();
// expressionLabel.setEditable(false);
GridPane numberGrid = new GridPane();
numberGrid.setHgap(5);
numberGrid.setVgap(5);
GridPane operatorGrid = new GridPane();
operatorGrid.setHgap(5);
operato
```
package calculator;
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.Group;
import javafx.scene.control.Label;
import javafx.scene.control.Button;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.GridPane;
import javafx.scene.layout.VBox;
import javafx.scene.layout.RowConstraints;
import javafx.scene.layout.ColumnConstraints;
import javafx.scene.paint.Color;
import javafx.event.ActionEvent;
import javafx.event.EventHandler;
import javafx.stage.Stage;
import javafx.geometry.HPos;
import java.util.ArrayList;
import javax.script.ScriptEngineManager;
import javax.script.ScriptEngine;
import javax.script.ScriptException;
public class Calculator extends Application{
private boolean operatorAlreadyPressed = false;
private boolean secondOperand = false;
private boolean scriptExceptionOccurred = false;
public static void main(String[] args){
Application.launch(args);
}
public void start(Stage primaryStage) throws ScriptException{
int windowWidth = 190;
int windowHeight = 250;
primaryStage.setTitle("Calculator");
primaryStage.setWidth(windowWidth);
primaryStage.setHeight(windowHeight);
// primaryStage.setResizable(false);
VBox root = new VBox();
Scene scene = new Scene(root, windowWidth, windowHeight, Color.WHITE);
Label expressionLabel = new Label();
// expressionLabel.setEditable(false);
GridPane numberGrid = new GridPane();
numberGrid.setHgap(5);
numberGrid.setVgap(5);
GridPane operatorGrid = new GridPane();
operatorGrid.setHgap(5);
operato
Solution
Good on you for taking on a project to learn, and even more for posting it here! Keep it up, you'll go far.
Make use of methods
Likely what you mean when you say modular, you should be using more methods. Right now pretty much all your logic is in your start method. Not only would using methods increase both maintainability and readability it would also make adding features more streamlined thus making your entire program both more flexible and extensible.
Here's a small example of where you can do this:
This if series where you currently check whether a deleted character is an operator:
You can extract this into the following:
This way you reduce your if condition to
Use Lambda expressions
Unless you're explicitly constrained in which runtime you may use, consider replacing your anonymous inner classes with lambda expressions.
e.g. Everywhere you use the
You can write it simply:
Read more about Lambda Expressions, from Oracle, here & for a general rundown of Java 8 features I recommend this.
Interface Intuitiveness
The 'C' button typically acts as a clear option on calculator. Yours currently acts as a backspace and you actually lack a clear option altogether.
In addition, after any result is displayed any proceeding input appends to the end of the previous result.
Lastly, I would organize things to be more like a natural calculator. It's fairly horizontal and there's quite a lot of space that just doesn't do anything. Good work on the flexible sizing however.
Remove commented code.
Commented is dead code. 'Nuff said.
On XML & CSS
A large appeal of JavaFX is the possibility to use CSS stylings and the ability to use XML to separate your model / view from your application logic. There is even software, like Scene Builder, that feature GUI whose use generates the desired XML for an interface. Here's a simple example using both CSS and XML facilitated by Scene Builder to design an interface.
Make use of methods
Likely what you mean when you say modular, you should be using more methods. Right now pretty much all your logic is in your start method. Not only would using methods increase both maintainability and readability it would also make adding features more streamlined thus making your entire program both more flexible and extensible.
Here's a small example of where you can do this:
This if series where you currently check whether a deleted character is an operator:
if(charToDelete == '+' || charToDelete == '-'
|| charToDelete == '*' || charToDelete == '/'){
operatorAlreadyPressed = false;
}You can extract this into the following:
private boolean isOperator(char potentialOperator) {
return potentialOperator == '+' || potentialOperator == '-' ||
potentialOperator == '*' || potentialOperator == '/';
}This way you reduce your if condition to
isOperator(charToDelete), You could even have a method that calls this one and alters operatorAlreadyPressed as a result. Try to see where else you can add more methods to your code!Use Lambda expressions
Unless you're explicitly constrained in which runtime you may use, consider replacing your anonymous inner classes with lambda expressions.
e.g. Everywhere you use the
setOnAction method, like the following:numberButtons.get(counter).setOnAction(new EventHandler(){
public void handle(ActionEvent e){
// logic
}
}You can write it simply:
numberButtons.get(counter).setOnAction(e -> {
// logic
});Read more about Lambda Expressions, from Oracle, here & for a general rundown of Java 8 features I recommend this.
Interface Intuitiveness
The 'C' button typically acts as a clear option on calculator. Yours currently acts as a backspace and you actually lack a clear option altogether.
In addition, after any result is displayed any proceeding input appends to the end of the previous result.
Lastly, I would organize things to be more like a natural calculator. It's fairly horizontal and there's quite a lot of space that just doesn't do anything. Good work on the flexible sizing however.
Remove commented code.
Commented is dead code. 'Nuff said.
On XML & CSS
A large appeal of JavaFX is the possibility to use CSS stylings and the ability to use XML to separate your model / view from your application logic. There is even software, like Scene Builder, that feature GUI whose use generates the desired XML for an interface. Here's a simple example using both CSS and XML facilitated by Scene Builder to design an interface.
Code Snippets
if(charToDelete == '+' || charToDelete == '-'
|| charToDelete == '*' || charToDelete == '/'){
operatorAlreadyPressed = false;
}private boolean isOperator(char potentialOperator) {
return potentialOperator == '+' || potentialOperator == '-' ||
potentialOperator == '*' || potentialOperator == '/';
}numberButtons.get(counter).setOnAction(new EventHandler<ActionEvent>(){
public void handle(ActionEvent e){
// logic
}
}numberButtons.get(counter).setOnAction(e -> {
// logic
});Context
StackExchange Code Review Q#118212, answer score: 3
Revisions (0)
No revisions yet.