patternjavaMinor
A "pointless" animation program
Viewed 0 times
pointlessanimationprogram
Problem
The assignment description for this code is this:
Each time the user clicks the Circles button, randomly colored circles
flow into the display and continuously change their shapes.
Here is a screenshot of my program running:
This is my first time seriously exploring the aspects of Java 8 development. Therefore, I would like reviews to be tailored as such, since I would like to know the best way for using it in the future.
```
package java8;
import java.util.stream.Stream;
import javafx.animation.ScaleTransition;
import javafx.animation.TranslateTransition;
import javafx.application.Application;
import javafx.geometry.Pos;
import javafx.scene.Node;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.layout.Pane;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
import javafx.scene.paint.Paint;
import javafx.scene.shape.Circle;
import javafx.stage.Stage;
import javafx.util.Duration;
/**
* A lab exercise to introduce Java 8 streams and JavaFX
* @author syb0rg
*/
public class Java8 extends Application {
public static final int ROWS = 4;
public static final int COLS = 5;
public static final int CELL_SIZE = 100;
public static final int RADIUS = 25;
/**
* This method kicks off the initialization process.
* @param primaryStage the main frame
*/
@Override
public void start(Stage primaryStage) {
root = new VBox();
canvas = new Pane();
starter = new Button("Circles");
configure();
addButtonHandler();
root.getChildren().addAll(canvas, starter);
primaryStage.setTitle("Java 8 Lab Exercise");
primaryStage.setScene(new Scene(root));
primaryStage.show();
}
/**
* This method sets the alignment of the root container
* and the size of the canvas.
*/
private void configure() {
root.setAlignment(Pos.CENTER);
canvas.setPrefSize(COLSCELL_SIZE, ROWSCELL_SIZE);
}
Each time the user clicks the Circles button, randomly colored circles
flow into the display and continuously change their shapes.
Here is a screenshot of my program running:
This is my first time seriously exploring the aspects of Java 8 development. Therefore, I would like reviews to be tailored as such, since I would like to know the best way for using it in the future.
```
package java8;
import java.util.stream.Stream;
import javafx.animation.ScaleTransition;
import javafx.animation.TranslateTransition;
import javafx.application.Application;
import javafx.geometry.Pos;
import javafx.scene.Node;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.layout.Pane;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
import javafx.scene.paint.Paint;
import javafx.scene.shape.Circle;
import javafx.stage.Stage;
import javafx.util.Duration;
/**
* A lab exercise to introduce Java 8 streams and JavaFX
* @author syb0rg
*/
public class Java8 extends Application {
public static final int ROWS = 4;
public static final int COLS = 5;
public static final int CELL_SIZE = 100;
public static final int RADIUS = 25;
/**
* This method kicks off the initialization process.
* @param primaryStage the main frame
*/
@Override
public void start(Stage primaryStage) {
root = new VBox();
canvas = new Pane();
starter = new Button("Circles");
configure();
addButtonHandler();
root.getChildren().addAll(canvas, starter);
primaryStage.setTitle("Java 8 Lab Exercise");
primaryStage.setScene(new Scene(root));
primaryStage.show();
}
/**
* This method sets the alignment of the root container
* and the size of the canvas.
*/
private void configure() {
root.setAlignment(Pos.CENTER);
canvas.setPrefSize(COLSCELL_SIZE, ROWSCELL_SIZE);
}
Solution
This looks very nice, straightforward code. A few minor improvements are possible.
Simplifications of lambdas
This uses a statement lambda:
... can be replaced with an expression lambda:
This uses an expression lambda:
... can be replaced with method reference:
Similarly:
Replace statement lambda with method reference:
There are a few other places too where you can apply similar transformations.
Redundant casts
In
... and others.
Random numbers
Instead of
Magic numbers
The number 500 appears at multiple places. If they are related, it would be good to put this into a constant.
Code organization
These variables are declared near the bottom of the cast:
The recommended practices is to declare all field variables at the top of the class.
Simplifications of lambdas
This uses a statement lambda:
starter.setOnAction(e -> {
circles();
});... can be replaced with an expression lambda:
starter.setOnAction(e -> circles());This uses an expression lambda:
Stream> circles = Stream.generate(() -> this.makeRow()).limit(ROWS);... can be replaced with method reference:
Stream> circles = Stream.generate(this::makeRow).limit(ROWS);Similarly:
circles.forEach(r -> {
addRowToCanvas(r);
});Replace statement lambda with method reference:
circles.forEach(this::addRowToCanvas);There are a few other places too where you can apply similar transformations.
Redundant casts
In
addToCanvas you have several redundant casts, that can be simply dropped:c.setFill((Paint) new Color(Math.random(), Math.random(), Math.random(), 1));
// ...
canvas.getChildren().add((Circle) c);
// ...... and others.
Random numbers
Instead of
Math.random(), it's better to use an instance of Random and its nextDouble() method. It's more modern, it can be seeded (for easier testing, for example), and passed as parameter, if ever needed.Magic numbers
The number 500 appears at multiple places. If they are related, it would be good to put this into a constant.
Code organization
These variables are declared near the bottom of the cast:
private VBox root;
private Pane canvas;
private Button starter;
private int row;
private int column;The recommended practices is to declare all field variables at the top of the class.
Code Snippets
starter.setOnAction(e -> {
circles();
});starter.setOnAction(e -> circles());Stream<Stream<Circle>> circles = Stream.generate(() -> this.makeRow()).limit(ROWS);Stream<Stream<Circle>> circles = Stream.generate(this::makeRow).limit(ROWS);circles.forEach(r -> {
addRowToCanvas(r);
});Context
StackExchange Code Review Q#126165, answer score: 4
Revisions (0)
No revisions yet.