11
\$\begingroup\$

I was looking through the JavaFX library, and I spotted that JavaFX DOES NOT HAVE A JOptionPane EQUIVALENT. (Or at least not in my version - as @Legato has said in comments, as of 8u40, there is a Dialog class. I use 8u25.) I decided the next step was to write a OptionPane class.

Since JOptionPane has three types of option panes:

  1. showInputDialog
  2. showMessageDialog
  3. showConfirmDialog

plus its Internal equivalents (e.g. showInternalConfirmDialog), I would have to display three different kinds of OptionPanes. If that was the case, then I would need three classes to use @Eric Stein's design (in this answer). So the question for me was, three good classes, or one bad class?

Three good classes was my choice.

Concerns:

  1. I'm fairly sure there's a clean way to combine this into one class, but I can't think of one. Any suggestions?
  2. Is this the best it can get?
  3. Does my code make sense?
  4. Anything else?

The code is below:

InputPane.java

import javafx.geometry.Insets;
import javafx.geometry.Pos;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.layout.HBox;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

public class InputPane {

    private static final MessagePane ILLEGAL_INPUT_PANE = new MessagePane.Builder("Illegal Input.").title("Illegal Input").build();

    private static final Insets MAIN_PANE_PADDING = new Insets(10, 20, 10, 20);
    private static final int MAIN_PANE_SPACING = 10;

    private static final Insets BUTTON_PANE_PADDING = new Insets(10, 50, 10, 50);
    private static final int BUTTON_PANE_SPACING = 20;

    private static final boolean IS_RESIZABLE = false;

    private static String inputResult = null;

    private final Stage stage = new Stage();

    private InputPane(final Builder builder) {
        inputResult = null;

        Label msgLabel = new Label(builder.message);

        TextField input = new TextField();

        Button okButton = new Button(builder.okButtonText);
        okButton.setOnAction(e -> {
            String text = input.getText();
            if (builder.inputType.isValidInput(text)) {
                inputResult = text;
                stage.close();
            } else {
                ILLEGAL_INPUT_PANE.showAndWait();
            }
        });

        HBox buttonPane = new HBox(BUTTON_PANE_SPACING);
        buttonPane.setPadding(BUTTON_PANE_PADDING);
        buttonPane.getChildren().add(okButton);

        if (builder.canCancel) {
            Button cancelButton = new Button(builder.cancelButtonText);
            buttonPane.getChildren().add(cancelButton);
            cancelButton.setOnAction(e -> stage.close());
        }

        VBox mainPane = new VBox(MAIN_PANE_SPACING);
        mainPane.setPadding(MAIN_PANE_PADDING);
        mainPane.setAlignment(Pos.CENTER);
        mainPane.getChildren().addAll(msgLabel, input, buttonPane);

        Scene scene = new Scene(mainPane);

        stage.setTitle(builder.title);
        stage.setScene(scene);
        stage.setResizable(IS_RESIZABLE);
        stage.setOnCloseRequest(e -> {
            if (builder.canClose) {
                stage.close();
            } else {
                e.consume();
            }
        });
    }

    public String showAndWait() {
        stage.showAndWait();
        String result = inputResult;
        inputResult = null;
        return result;
    }

    public static final class Builder {

        private String message;
        private String title = "Input";
        private String okButtonText = "OK";
        private String cancelButtonText = "Cancel";
        private InputType inputType = InputType.STRING;
        private boolean canCancel = true;
        private boolean canClose = true;

        public Builder(String message) {
            this.message = message;
        }

        public Builder title(final String title) {
            this.title = title;
            return this;
        }

        public Builder okButtonText(final String okButtonText) {
            this.okButtonText = okButtonText;
            return this;
        }

        public Builder cancelButtonText(final String cancelButtonText) {
            this.cancelButtonText = cancelButtonText;
            return this;
        }

        public Builder inputType(final InputType inputType) {
            this.inputType = inputType;
            return this;
        }

        public Builder canCancel(final boolean canCancel) {
            this.canCancel = canCancel;
            return this;
        }

        public Builder canClose(final boolean canClose) {
            this.canClose = canClose;
            return this;
        }

        public InputPane build() {
            return new InputPane(this);
        }

    }

}

MessagePane.java

import javafx.geometry.Insets;
import javafx.geometry.Pos;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

public class MessagePane {

    private static final Insets MAIN_PANE_PADDING = new Insets(10, 20, 10, 20);
    private static final int MAIN_PANE_SPACING = 10;

    private static final boolean IS_RESIZABLE = false;

    private final Stage stage = new Stage();

    private MessagePane(final Builder builder) {
        Label msgLabel = new Label(builder.message);

        Button okButton = new Button(builder.okButtonText);
        okButton.setOnAction(e -> {
            stage.close();
        });

        VBox mainPane = new VBox(MAIN_PANE_SPACING);
        mainPane.setPadding(MAIN_PANE_PADDING);
        mainPane.setAlignment(Pos.CENTER);
        mainPane.getChildren().addAll(msgLabel, okButton);

        Scene scene = new Scene(mainPane);

        stage.setTitle(builder.title);
        stage.setScene(scene);
        stage.setResizable(IS_RESIZABLE);
        stage.setOnCloseRequest(e -> {
            if (builder.canClose) {
                stage.close();
            } else {
                e.consume();
            }
        });
    }

    public void show() {
        stage.show();
    }

    public void showAndWait() {
        stage.showAndWait();
    }

    public static final class Builder {

        private String message;
        private String title = "Input";
        private String okButtonText = "OK";
        private boolean canClose = true;

        public Builder(String message) {
            this.message = message;
        }

        public Builder title(final String title) {
            this.title = title;
            return this;
        }

        public Builder okButtonText(final String okButtonText) {
            this.okButtonText = okButtonText;
            return this;
        }

        public Builder canClose(final boolean canClose) {
            this.canClose = canClose;
            return this;
        }

        public MessagePane build() {
            return new MessagePane(this);
        }

    }

}

ConfirmPane.java

import javafx.geometry.Insets;
import javafx.geometry.Pos;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.layout.HBox;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

public class ConfirmPane {

    private static final Insets MAIN_PANE_PADDING = new Insets(10, 20, 10, 20);
    private static final int MAIN_PANE_SPACING = 10;

    private static final Insets BUTTON_PANE_PADDING = new Insets(10, 50, 10, 50);
    private static final int BUTTON_PANE_SPACING = 20;

    private static final boolean IS_RESIZABLE = false;

    private static boolean confirmResult = false;

    private final Stage stage = new Stage();

    private ConfirmPane(final Builder builder) {
        Label msgLabel = new Label(builder.message);

        TextField input = new TextField();

        Button okButton = new Button(builder.okButtonText);
        okButton.setOnAction(e -> {
            confirmResult = true;
            stage.close();
        });

        HBox buttonPane = new HBox(BUTTON_PANE_SPACING);
        buttonPane.setPadding(BUTTON_PANE_PADDING);
        buttonPane.getChildren().add(okButton);

        Button cancelButton = new Button(builder.cancelButtonText);
        buttonPane.getChildren().add(cancelButton);
        cancelButton.setOnAction(e -> stage.close());

        VBox mainPane = new VBox(MAIN_PANE_SPACING);
        mainPane.setPadding(MAIN_PANE_PADDING);
        mainPane.setAlignment(Pos.CENTER);
        mainPane.getChildren().addAll(msgLabel, input, buttonPane);

        Scene scene = new Scene(mainPane);

        stage.setTitle(builder.title);
        stage.setScene(scene);
        stage.setResizable(IS_RESIZABLE);
        stage.setOnCloseRequest(e -> {
            if (builder.canClose) {
                stage.close();
            } else {
                e.consume();
            }
        });
    }

    public boolean showAndWait() {
        stage.showAndWait();
        boolean result = confirmResult;
        confirmResult = false;
        return result;
    }

    public static final class Builder {

        private String message;
        private String title = "Input";
        private String okButtonText = "OK";
        private String cancelButtonText = "Cancel";
        private boolean canClose = false;

        public Builder(String message) {
            this.message = message;
        }

        public Builder title(final String title) {
            this.title = title;
            return this;
        }

        public Builder okButtonText(final String okButtonText) {
            this.okButtonText = okButtonText;
            return this;
        }

        public Builder cancelButtonText(final String cancelButtonText) {
            this.cancelButtonText = cancelButtonText;
            return this;
        }

        public Builder canClose(final boolean canClose) {
            this.canClose = canClose;
            return this;
        }

        public ConfirmPane build() {
            return new ConfirmPane(this);
        }

    }

}
\$\endgroup\$
  • \$\begingroup\$ As of 8u40, there's a Dialog class under Scene control. \$\endgroup\$ – Legato Oct 21 '15 at 22:29
  • \$\begingroup\$ @Legato I see now, but I'll pretend that I never saw your comment and stick to my own classes (because I worked hard doing this). Don't take any offence; it's just that I prefer my own classes. \$\endgroup\$ – TheCoffeeCup Oct 21 '15 at 22:37
2
\$\begingroup\$

Bug or Feature?

ConfirmPane is created with a TextField input, which makes the pane almost identical with InputPane. However, the aim of ConfirmPane is to receive only 'Yes' or 'No' from the user and the text input field will disturb its user. How could it appear there? It looks like there was some copy-pasting!

Duplicate Code

Many, many items are present in more than one class: most of private static final constants, HBox and VBox and stage objects initialization.

Since they are common for the three classes, they may be easily extracted into something like public abstract class AbstractPane. The constants will be moved there directly. mainPane instances may be created with its dedicated method:

protected VBox buildMainPaneWith(Node... nodes) {
    VBox mainPane = new VBox(MAIN_PANE_SPACING);
    mainPane.setPadding(MAIN_PANE_PADDING);
    mainPane.setAlignment(Pos.CENTER);
    mainPane.getChildren().addAll(nodes);
    return mainPane;
}

The same principle may be applied to HBox buttonPane and stage initialization.

Return Values

ConfirmPane and InputPane are used to collect user input. A static variable is probably the worst choice for keeping the value to return. Just remove the modifier, and it'll be better.

For InputPane there is no need to introduce the inputResult field. There is already a TextField for that. Declare private TextField input at class level and showAndWait() method will be reduced to the following:

public String showAndWait() {
    stage.showAndWait();
    return this.input.getText();
}

However, for ConfirmPane the private boolean confirmResult should be kept, since there is no UI element that holds the value.

What About Modal Dialogs?

There is one important issue concerning the solution that you implemented. Often, applications need to show a modal dialog (that blocks its preceeding windows until the user gives some input). Currently, there is no way to plug the panes as modal dialogs.

A solution for this issue could be found by providing the owner object through the Builders and calling stage.initOwner(arg) and stage.initModality(arg) where appropriate.

Refactor the Builders

The Builder definitions contain repetitive

private String message;
private String title;
private String okButtonText;
private boolean canClose;

It looks very ugly and should be extracted. It will be a bit difficult to implement it by using a hierarchy similar to the Panes, because of chained method calls. But this can also be done by implementing a single builder for all your Panes, where there will be a way to set the type of the pane to build.

After all these refactorings, you'll see that the total number of lines of code will be reduced at least by half.

\$\endgroup\$
3
\$\begingroup\$

I'm not sure I like these Builder nested classes. Oh sure, like the rest of the code you're showing us, it's very nice & tidy, readable, consistent and well-formatted code.

That builder looks like a builder, but in reality this:

ConfirmPane builder = new Builder("foo").title("title")
                                        .okButtonText("ok")
                                        .cancelButtonText("cancel")
                                        .canClose(false)
                                        .build();

Is IMO pretty much a fancy/look-ma/convoluted way to do this:

ConfirmPaneModel = new ConfirmPaneModel("foo", "title", "ok", "cancel", false);

What happens if you do this?

ConfirmPane builder = new Builder("foo").title("title")
                                        .okButtonText("ok")
                                        .okButtonText("foobarred")
                                        .cancelButtonText("cancel")
                                        .okButtonText("foobarred")
                                        .title("foobarred")
                                        .canClose(false)
                                        .canClose(true)
                                        .build();

That's right. You're foobarred. The Builder Pattern works much better when there's no specific order or number of times a method could or should be called before build(). When a method can only ever be called once per "builder" for the result to make sense, you're looking at an attribute of your "builder" type, a property. Property. Get. Set. We love properties, they're so unsurprising!

ConfirmPaneModel model = new confirmPaneModel();
model.title = "title";
model.okButtonText = "ok";
model.cancelButtonText = "cancel";
model.canClose = false;

Almost boring :-)

\$\endgroup\$
  • \$\begingroup\$ Can you please provide an example on how you would do this? Thanks. \$\endgroup\$ – TheCoffeeCup Oct 23 '15 at 2:45
  • 1
    \$\begingroup\$ @MannyMeng in C# I'd do public string Title { get; set; } - I believe in Java it's slightly more verbose.. \$\endgroup\$ – Mathieu Guindon Oct 23 '15 at 2:56
  • \$\begingroup\$ I'm not sure what the problem with the buildpattern is in this case. AFAIK this is how the builder pattern normally works, by simply setting a property and returning itself. If you call .title(...) twice, suit yourself :) \$\endgroup\$ – Simon Forsberg Oct 28 '15 at 10:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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