Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I created this JavaFX code:

final String change[] =
    {
        "Full Screen", "Exit Full Screen"
    };
    final MenuItem fullScreen = MenuItemBuilder.create().text(change[0]).build();

    fullScreen.setOnAction(new EventHandler<ActionEvent>()
    {
        @Override
        public void handle(ActionEvent e)
        {
            fullScreen.setText((fullScreen.getText().equals(change[0])) ? change[1] : change[0]);

            if (fullScreen.getText().equals(change[0]))
            {

                primaryStage.setFullScreen(false);
            }
            else
            {

                primaryStage.setFullScreen(true);
            }

        }
    });

    view.getItems().add(fullScreen);

I need to improve it by using switch statement in which I want to set the text and setFullScreen. Is there any more elegant way to implement this?

EDIT This code works but the implementation is very ugly.

share|improve this question

migrated from programmers.stackexchange.com Jul 15 '13 at 12:30

This question came from our site for professional programmers interested in conceptual questions about software development.

1 Answer 1

This can be made a lot more elegant with the State Pattern.

We'll introduce an enum for the two states :

private static enum Mode {
    FULLSCREEN("Exit Full Screen"), NORMAL("Full Screen");

    private final String toggleActionText;

    private Mode(String text) {
        this.toggleActionText = text;
    }

    public String getToggleActionText() {
        return toggleActionText;
    }

    public boolean isFullScreen() {
        return FULLSCREEN == this;
    }

    public Mode other() {
        return isFullScreen() ? NORMAL : FULLSCREEN;
    }
}

Then, having added a properly initialized Mode field to the class, you can simplify your code to this :

final MenuItem fullScreen = MenuItemBuilder.create().text(mode.getToggleActionText()).build();

fullScreen.setOnAction(new EventHandler<ActionEvent>()
{
    @Override
    public void handle(ActionEvent e)
    {
        mode = mode.other();
        fullScreen.setText(mode.getToggleActionText());
        primaryStage.setFullScreen(mode.isFullScreen());
    }
});

view.getItems().add(fullScreen);

As you can see, it is a lot simpler to delegate the state dependent behavior to the Mode instance than to use an if whenever you have state dependent behavior.

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.