patternjavaMinor
What makes a hero?
Viewed 0 times
makesherowhat
Problem
This is a simple program meant to show how dynamic JavaFX is. It moves one list item to another and back without having to update the screen directly, thanks to the Observable Collection classes. It also exemplifies how links would work.
In the same vein as Hello parallel world, I practiced and illustrated some ideas with the intention of teaching a friend later on. I'm curious about what could be improved, and if anything is done poorly or inefficiently.
```
import java.awt.Desktop;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import javafx.application.Application;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.geometry.HPos;
import javafx.geometry.Insets;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.control.ListView;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.ColumnConstraints;
import javafx.scene.layout.GridPane;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
public class Heroes extends Application {
public static void main(String[] args) {
launch(args);
}
@Override
public void start(Stage stage) {
ColumnConstraints column1 = new ColumnConstraints(150, 150, Double.MAX_VALUE);
column1.setHgrow(Priority.ALWAYS);
ColumnConstraints column2 = new ColumnConstraints(60);
ColumnConstraints column3 = new ColumnConstraints(150, 150, Double.MAX_VALUE);
column3.setHgrow(Priority.ALWAYS);
GridPane gridpane = new GridPane();
gridpane.setPadding(new Insets(5));
gridpane.setHgap(10);
gridpane.setVgap(10);
gridpane.getColumnConstraints().addAll(column1, column2, column3);
Label candidatesLabel = new Label("Candidates");
GridPane.setHalignment(candidatesLabel, HPos.CENTER);
gridpane.add(candidatesLabel, 0, 0);
In the same vein as Hello parallel world, I practiced and illustrated some ideas with the intention of teaching a friend later on. I'm curious about what could be improved, and if anything is done poorly or inefficiently.
```
import java.awt.Desktop;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import javafx.application.Application;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.geometry.HPos;
import javafx.geometry.Insets;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.control.ListView;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.ColumnConstraints;
import javafx.scene.layout.GridPane;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
public class Heroes extends Application {
public static void main(String[] args) {
launch(args);
}
@Override
public void start(Stage stage) {
ColumnConstraints column1 = new ColumnConstraints(150, 150, Double.MAX_VALUE);
column1.setHgrow(Priority.ALWAYS);
ColumnConstraints column2 = new ColumnConstraints(60);
ColumnConstraints column3 = new ColumnConstraints(150, 150, Double.MAX_VALUE);
column3.setHgrow(Priority.ALWAYS);
GridPane gridpane = new GridPane();
gridpane.setPadding(new Insets(5));
gridpane.setHgap(10);
gridpane.setVgap(10);
gridpane.getColumnConstraints().addAll(column1, column2, column3);
Label candidatesLabel = new Label("Candidates");
GridPane.setHalignment(candidatesLabel, HPos.CENTER);
gridpane.add(candidatesLabel, 0, 0);
Solution
2 male and 2 female superheros... Very politically correct ;-)
I suggest to try to avoid repeated code as much as possible.
For example,
only the URL parameter is different.
I would refactor this in a way that the
so that you can write the
after the
Of course, this won't apply to Code Reviewers.
In that case,
instead of setting a URL.
Another kind of duplication in your code is that of the names of superheros:
they appear first when you add them to the
and then again in the
As such, it seems it would be better to use a
and do without the
I suggest to try to avoid repeated code as much as possible.
For example,
Desktop.getDesktop().browse(...) is used in most of the case statements,only the URL parameter is different.
I would refactor this in a way that the
case statements set a URL variable,so that you can write the
Desktop.getDesktop().browse(...) call one time,after the
switch.Of course, this won't apply to Code Reviewers.
In that case,
return from the switch,instead of setting a URL.
Another kind of duplication in your code is that of the names of superheros:
they appear first when you add them to the
candidates list,and then again in the
case statements.As such, it seems it would be better to use a
Map (or Map,and do without the
switch.Context
StackExchange Code Review Q#93572, answer score: 7
Revisions (0)
No revisions yet.