Search code examples
javajavafxcontrollerfxmlscenebuilder

What is good coding practice when it comes to structuring a JavaFX controller?


I am a student learning how to use JavaFX and I've got my first GUI working by using SceneBuilder and a Controller class. However, from my point of view the structure of the code in the controller looks incredibly messy and ugly because I put every event handler in the initialize() method of Controller. This makes it look like this:

@FXML
private void initialize() {
    dealtHandLabel.setText("Your cards will be shown here.");
    TextInputDialog userInput = new TextInputDialog();
    userInput.setTitle("How many cards?");
    userInput.setHeaderText("Enter how many cards you want to get.");
    userInput.setContentText("No. of cards:");

    //This makes it so that the button displays a hand of cards (of specified amount) when clicked
    dealHand.setOnAction(event -> {
        Optional<String> result = userInput.showAndWait();
        if(result.isPresent()) {
            int requestedAmount = Integer.parseInt(result.get());
            StringBuilder sb = new StringBuilder();
            cardHand = deck.dealHand(requestedAmount);
            cardHand.forEach((card) -> sb.append(card.getAsString()).append(" "));
            dealtHandLabel.setText(sb.toString());
        }
    });

    //This button uses lambdas and streams to display requested information (sum, heart cards, etc.)
    checkHand.setOnAction(event -> {
        int cardSum = cardHand.stream().mapToInt(card -> card.getFace()).sum();
        List<PlayingCard> spadeCards = cardHand.stream().filter((card) -> card.getSuit() == 'S').toList();
        List<PlayingCard> heartCards = cardHand.stream().filter((card) -> card.getSuit() == 'H').toList();
        List<PlayingCard> diamondCards = cardHand.stream().filter((card) -> card.getSuit() == 'D').toList();
        List<PlayingCard> clubCards = cardHand.stream().filter((card) -> card.getSuit() == 'C').toList();
        StringBuilder sb = new StringBuilder();
        heartCards.forEach((card) -> sb.append(card.getAsString()).append(" "));

        sumOfFacesField.setText(String.valueOf(cardSum));
        heartCardsField.setText(sb.toString());

        if(heartCards.size() >= 5 || diamondCards.size() >= 5 || spadeCards.size() >= 5 || clubCards.size() >= 5) {
            flushField.setText("Yes");
        }
        else {
            flushField.setText("No");
        }
        if(cardHand.stream().anyMatch((card) -> card.getAsString().equals("S12"))) {
            spadesQueenField.setText("Yes");
        }
        else {
            spadesQueenField.setText("No");
        }
    });
}

My lecturer does the exact same thing where he straight up puts every node handler into the initialize method, but I am not sure if this is good coding practice because it makes code harder to read from my point of view. Would it be better to put the different handlers into separate methods and connect them to the correct nodes using SceneBuilder, or is putting everything into initialize considered common coding practice among JavaFX developers?


Solution

  • This is an opinionated, perhaps even arbitrary decision, both approaches are OK.

    There is nothing wrong with coding the event handlers in the initialize function versus referencing an event method handler from FXML.

    These kinds of things are out of usually out of scope for StackOverflow, but I'll add some pointers and opinions anyway as it may help you or others regardless of StackOverflow policy.

    Reference the actions in Scene Builder

    Personally, I'd reference the action in Scene Builder:

    • Fill in a value for onAction or other events in the code panel section of Scene Builder UI for the highlighted node.

    This will also add the reference in FXML, so you will have something like this, with the hashtag value for the onAction attribute:

    <Button fx:id="saveButton" mnemonicParsing="false" onAction="#save" text="Save" />
    

    Have Scene Builder generate a skeleton (View | Show Sample Skeleton). This will create a method signature to fill in, like this:

    @FXML
    void save(ActionEvent event) {
    
    }
    

    Then place the event handling code in there.

    For that setup, IDEs such as Idea will do additional intelligent checks for consistency and allow element navigation between FXML and Java code, which can be nice, though that isn't really critical.


    What follows is optional additional information on some important design decisions regarding JavaFX controllers.

    Ignore this if it confuses you or is not relevant for your application (which is likely for a small study application).

    Consider using MVC and a shared model

    The more important design decision with regards to controllers is, usually, whether or not to use a shared model and MVC, MVP, or MVVM.

    I'd encourage you to study that, research the acronyms, and look at the Eden coding MVC guide.

    Consider using dependency injection

    Also consider whether or not to use a dependency injection framework with the controllers, e.g. Spring integration (for a more complex app) or the clever eden injection pattern. You don't need to use these patterns, but they can help. Spring in particular is complex, and the integration with JavaFX is currently a bit tricky. I know both, so I would use them if the app called for it, but for others, it may not be a good combination.

    Consider a business services layer

    For medium to larger sized apps, in addition to having a separate model layer, try to separate the business logic out of the controller so the controller is just dealing with invoking functions on business services that manipulate the shared model and binding that model to the UI, rather than directly implementing the business logic in the controller.

    This makes reusing, reasoning about, and testing the business logic easier. For smaller apps, the additional abstraction is not necessary and you can do the work in the controller.

    Often such a handler will call an injected service that interacts with the shared model. If the updated data also needs to be persisted, then the injected service can also invoke a database access object or rest API client to persist the data.

    Putting it all together

    So to go back to the prior example, you might implement your save function in the controller like this:

    public class UserController {
    
        @FXML
        TextField userName;
    
        private UserService userService;
        @Autowired
        public void setUserService(UserService userService) {
            userService = userService;
        }
    
        @FXML
        void save(ActionEvent event) {
            userService.saveUser(new User(userName.getText())); 
        }
    
    }
    

    Where the userService might reference a Spring WebFlux REST Client to persist the new user to a cloud-deployed REST service or maybe a Spring Data DAO to store the new user in a shared RDBMS database.

    As noted, not all apps need this level of abstraction. Especially, the injection frameworks that are not required for small apps. And you can mix architectural styles within a given app, using shared models and services as appropriate and writing some smaller functions directly in the controller if you prefer to code that way. Just be careful if you do mix design patterns that it doesn't end up a jumbled mess :-)