Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have decided to create a dashboard view of a number of running processes/programs/applications, because it became tedious to monitor a bunch of applications, which I had to boot up manually.

Please keep in mind that the .fxml files have been autogenerated using JavaFX SceneBuilder 2.0 and hence may not have proper formatting, but are necessary if you want to run the program.

I'd like a review on all aspects and the design may also be reviewed if you wish, but I'm not a GUI designer.

Dashboard.java

public class Dashboard extends Application {
    @Override
    public void start(Stage stage) throws Exception {
        Parent root = FXMLLoader.load(getClass().getResource("Dashboard.fxml"));

        Scene scene = new Scene(root);
        stage.setScene(scene);
        stage.setTitle("DPC2 Dashboard");
        stage.show();
    }

    /**
     * @param args the command line arguments
     */
    public static void main(String[] args) {
        launch(args);
    }
}

DashboardTabPaneController.java

public class DashboardTabPaneController {
    @FXML
    private Button button;

    @FXML
    private TextArea textArea;

    public Button getButton() {
        return button;
    }

    public TextArea getTextArea() {
        return textArea;
    }
}

Server.java

public class Server {
    private final String name;
    private final List<String> commands;
    private final ProcessBuilder processBuilder;

    private boolean running = false;
    private Runnable startHandler = () -> {};
    private Runnable stopHandler = () -> {};

    public Server() {
        this.name = null;
        this.commands = null;
        this.processBuilder = null;
    }
    private Process process;

    public Server(final String name, final List<String> commands) {
        this.name = Objects.requireNonNull(name, "name");
        this.commands = Objects.requireNonNull(commands, "commands");
        this.processBuilder = new ProcessBuilder(commands);
    }

    public void setStartHandler(final Runnable startHandler) {
        this.startHandler = Objects.requireNonNull(startHandler, "startHandler");
    }

    public void setStopHandler(final Runnable stopHandler) {
        this.stopHandler = Objects.requireNonNull(stopHandler, "stopHandler");
    }

    public String getName() {
        return name;
    }

    public List<String> getCommands() {
        return commands;
    }

    public boolean isRunning() {
        return running;
    }

    public Process getProcess() {
        return process;
    }

    public void start() {
        if (running) {
            throw new IllegalStateException("server " + name + " is already running");
        }
        try {
            process = processBuilder.start();
            setRunning(true);
        } catch (IOException ex) {
            throw new UncheckedIOException(ex);
        }
        new Thread(() -> {
            try {
                process.waitFor();
            } catch (InterruptedException ex) {
                Thread.currentThread().interrupt();
            }
            setRunning(false);
        }).start();
    }

    public void stop() {
        if (!running) {
            throw new IllegalStateException("server " + name + " is not running");
        }
        process.destroyForcibly();
        try {
            process.waitFor();
        } catch (InterruptedException ex) {
            Thread.currentThread().interrupt();
        }
        setRunning(false);
    }

    private void setRunning(final boolean value) {
        running = value;
        if (value) {
            startHandler.run();
        }
        else {
            stopHandler.run();
        }
    }
}

DashboardController.java

public class DashboardController implements Initializable {
    private final static String GENERAL_BUTTON_ENABLE = "Start all servers";
    private final static String GENERAL_BUTTON_DISABLE = "Stop all servers";

    private final static String SERVER_BUTTON_ENABLE = "Start server";
    private final static String SERVER_BUTTON_DISABLE = "Stop server";

    private final static List<Server> DEFAULT_SERVERS = new ArrayList<>();
    static {
        DEFAULT_SERVERS.add(new Server("Ping Test", Arrays.asList("C:/Windows/System32/ping", "localhost")));
    }

    private final static DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss");

    private final List<Server> servers = new ArrayList<>();
    private final Map<Server, Tab> serverTabs = new HashMap<>();
    private final Map<Server, DashboardTabPaneController> serverControllers = new HashMap<>();

    public DashboardController() {
        this.servers.addAll(DEFAULT_SERVERS);
    }

    @FXML
    private TabPane tabPane;

    @FXML
    private Button generalButton;

    @FXML
    private TextArea textArea;

    @Override
    public void initialize(final URL url, final ResourceBundle resourceBundle) {
        generalButton.setText(GENERAL_BUTTON_ENABLE);
        servers.forEach(this::createTab);
        servers.forEach(server -> {
            server.setStartHandler(() -> {
                Platform.runLater(() -> writeToGeneralTextArea("Started server " + server.getName() + "."));
                Platform.runLater(() -> serverControllers.get(server).getButton().setText(SERVER_BUTTON_DISABLE));
                if (servers.stream().anyMatch(Server::isRunning)) {
                    Platform.runLater(() -> generalButton.setText(GENERAL_BUTTON_DISABLE));
                }
            });
            server.setStopHandler(() -> {
                Platform.runLater(() -> writeToGeneralTextArea("Stopped server " + server.getName() + "."));
                Platform.runLater(() -> serverControllers.get(server).getButton().setText(SERVER_BUTTON_ENABLE));
                if (servers.stream().noneMatch(Server::isRunning)) {
                    Platform.runLater(() -> generalButton.setText(GENERAL_BUTTON_ENABLE));
                }
            });
        });
    }

    private void createTab(final Server server) {
        Tab tab = new Tab(server.getName());
        try {
            FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource("DashboardTabPane.fxml"));
            Node dashboardTabPane = fxmlLoader.load();
            DashboardTabPaneController controller = fxmlLoader.getController();
            controller.getButton().setText(SERVER_BUTTON_ENABLE);
            controller.getButton().addEventHandler(MouseEvent.MOUSE_CLICKED, mouseEvent -> handleServerButtonAction(server, controller.getButton(), mouseEvent));
            serverControllers.put(server, controller);
            tab.setContent(dashboardTabPane);
        } catch (IOException ex) {
            throw new UncheckedIOException(ex);
        }
        serverTabs.put(server, tab);
        tabPane.getTabs().add(tab);
    }

    @FXML
    private void handleGeneralButtonAction(final MouseEvent mouseEvent) {
        switch (generalButton.getText()) {
            case GENERAL_BUTTON_ENABLE:
                generalButton.setText(GENERAL_BUTTON_DISABLE);
                startAllServers();
                break;
            case GENERAL_BUTTON_DISABLE:
                generalButton.setText(GENERAL_BUTTON_ENABLE);
                stopAllServers();
                break;
            default:
                throw new IllegalStateException("Impossible string for " + generalButton + " / string = " + generalButton.getText());
        }
    }

    private void handleServerButtonAction(final Server server, final Button button, final MouseEvent mouseEvent) {
        switch (button.getText()) {
            case SERVER_BUTTON_ENABLE:
                startServer(server);
                break;
            case SERVER_BUTTON_DISABLE:
                stopServer(server);
                break;
            default:
                throw new IllegalStateException("Impossible string for " + button + " / string = " + button.getText());
        }
    }

    private void startAllServers() {
        servers.stream()
                .filter(server -> !server.isRunning())
                .forEach(this::startServer);
    }

    private void stopAllServers() {
        servers.stream()
                .filter(server -> server.isRunning())
                .forEach(this::stopServer);
    }

    private void startServer(final Server server) {
        server.start();
        connectServerToTextArea(server, server.getProcess().getInputStream());
        connectServerToTextArea(server, server.getProcess().getErrorStream());
    }

    private void stopServer(final Server server) {
        server.stop();
    }

    private void writeToGeneralTextArea(final String text) {
        Platform.runLater(() -> textArea.appendText("[" + DATE_TIME_FORMATTER.format(LocalDateTime.now()) + "] " + text + System.lineSeparator()));
    }

    private void connectServerToTextArea(final Server server, final InputStream inputStream) {
        Objects.requireNonNull(inputStream, "inputStream");
        BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(inputStream));
        new Thread(() -> {
            bufferedReader.lines()
                    .forEach(line -> Platform.runLater(() -> serverControllers.get(server).getTextArea().appendText(line + System.lineSeparator())));
        }).start();
    }
}

DashboardTabPane.fxml

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.geometry.*?>
<?import java.lang.*?>
<?import java.util.*?>
<?import javafx.scene.*?>
<?import javafx.scene.control.*?>
<?import javafx.scene.layout.*?>

<AnchorPane minHeight="0.0" minWidth="0.0" prefHeight="180.0" prefWidth="200.0" xmlns="http://javafx.com/javafx/8" xmlns:fx="http://javafx.com/fxml/1" fx:controller="dpc2dashboard.DashboardTabPaneController">
   <children>
      <BorderPane fx:id="borderPane" prefHeight="200.0" prefWidth="200.0" AnchorPane.bottomAnchor="0.0" AnchorPane.leftAnchor="0.0" AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0">
         <center>
            <ScrollPane fx:id="scrollPane" fitToHeight="true" fitToWidth="true" prefHeight="200.0" prefWidth="200.0" BorderPane.alignment="CENTER">
               <content>
                  <AnchorPane minHeight="0.0" minWidth="0.0" prefHeight="200.0" prefWidth="200.0">
                     <children>
                        <TextArea fx:id="textArea" editable="false" layoutX="7.0" prefHeight="200.0" prefWidth="200.0" wrapText="true" AnchorPane.bottomAnchor="0.0" AnchorPane.leftAnchor="0.0" AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0" />
                     </children>
                  </AnchorPane>
               </content>
               <BorderPane.margin>
                  <Insets bottom="5.0" left="5.0" right="5.0" top="5.0" />
               </BorderPane.margin>
            </ScrollPane>
         </center>
         <top>
            <Button fx:id="button" mnemonicParsing="false" text="Button" BorderPane.alignment="TOP_LEFT">
               <BorderPane.margin>
                  <Insets left="5.0" top="5.0" />
               </BorderPane.margin>
            </Button>
         </top>
      </BorderPane>
   </children>
</AnchorPane>

Dashboard.fxml

<?xml version="1.0" encoding="UTF-8"?>

<?import java.lang.*?>
<?import javafx.geometry.*?>
<?import javafx.scene.control.*?>
<?import javafx.scene.layout.*?>

<TabPane fx:id="tabPane" maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="600.0" prefWidth="800.0" tabClosingPolicy="UNAVAILABLE" xmlns="http://javafx.com/javafx/8" xmlns:fx="http://javafx.com/fxml/1" fx:controller="dpc2dashboard.DashboardController">
  <tabs>
    <Tab text="General">
         <content>
            <javafx.scene.layout.AnchorPane minHeight="0.0" minWidth="0.0" prefHeight="180.0" prefWidth="200.0">
               <children>
                  <BorderPane prefHeight="200.0" prefWidth="200.0" AnchorPane.bottomAnchor="0.0" AnchorPane.leftAnchor="0.0" AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0">
                     <center>
                        <ScrollPane fitToHeight="true" fitToWidth="true" prefHeight="200.0" prefWidth="200.0" BorderPane.alignment="CENTER">
                           <content>
                              <AnchorPane minHeight="0.0" minWidth="0.0" prefHeight="200.0" prefWidth="200.0">
                                 <children>
                                    <TextArea fx:id="textArea" editable="false" layoutX="7.0" prefHeight="200.0" prefWidth="200.0" wrapText="true" AnchorPane.bottomAnchor="0.0" AnchorPane.leftAnchor="0.0" AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0" />
                                 </children>
                              </AnchorPane>
                           </content>
                           <BorderPane.margin>
                              <Insets bottom="5.0" left="5.0" right="5.0" top="5.0" />
                           </BorderPane.margin>
                        </ScrollPane>
                     </center>
                     <top>
                        <Button fx:id="generalButton" mnemonicParsing="false" onMouseClicked="#handleGeneralButtonAction" text="Button" BorderPane.alignment="TOP_LEFT">
                           <BorderPane.margin>
                              <Insets left="5.0" top="5.0" />
                           </BorderPane.margin>
                        </Button>
                     </top>
                  </BorderPane>
               </children>
            </javafx.scene.layout.AnchorPane>
         </content></Tab>
  </tabs>
</TabPane>

Currently it connects to the ping process, as I still need to figure out how to run my applications correctly outside of the IDE.

share|improve this question
up vote 5 down vote accepted

JavaFX Related

  1. main() method : in Dashboard.java You do not need it. It's an IDE bug, It's only required for old IDEs. (I recommend using the latest NetBeans, version 8) from Oracle

    The main() method is not required for JavaFX applications when the JAR file for the application is created with the JavaFX Packager tool, which embeds the JavaFX Launcher in the JAR file. However, it is useful to include the main() method so you can run JAR files that were created without the JavaFX Launcher, such as when using an IDE in which the JavaFX tools are not fully integrated. Also, Swing applications that embed JavaFX code require the main() method.

  2. Use Toggle Buttons : in DashboardController.java you are doing
    switch (generalButton.getText()) I suggest using ToggleButtons instead. It's built for toggling after all. from Oracle

    Two or more toggle buttons can be combined into a group where only one button at a time can be selected


Other

  1. Class Names : The class Server can be refactored to something that reveals it's intent. I suggest Process. from Clean Code

    It is easy to say that names should reveal intent.

  2. Package Names : you are using package name dpc2dashboard which I think can be refactored to dashboard which would make more sense and more pronounceable. from Clean Code

    Humans are good at words. A significant part of our brains is dedicated to the concept of words. And words are, by definition, pronounceable. It would be a shame not to take advantage of that huge portion of our brains that has evolved to deal with spoken lan- guage. So make your names pronounceable.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.