(JavaFx) Proper way represent static FXML fields (and why this is a bad idea)

Question

I would like to have a FXML field accessible from all classes. In a perfect world, I would just do this:

public class CanvasController {

    @FXML
    private static final AnchorPane canvas; // This is wrong

    public static AnchorPane getCanvas() {
        return canvas;
    }
}

However, as explained by this similar question, static fields are a "terrible idea".

I'm designing a node editor, something like this:

I want to add a node, as a child of this AnchorPane, from anywhere in my project.

Am I wrong in wanting to do this? Why are static fields a bad idea, and how should I do this instead?

(The similar question recommends using a property, but JavaFx doesn't have a NodeProperty or AnchorPaneProperty).

score 2 · Accepted Answer · edited May 23 '17 at 10:29

This question is probably too opinion-based for this forum, but I think the opinions I'm expressing here are held widely enough for this to be acceptable.

Why are static fields a bad idea

Suppose you did what you are trying to do here, and it was allowed to work. Imagine your boss comes into your office in 6 months, and says something like "The customers love the application, but they want to be able to open multiple node editors at the same time. Can you make the application so they can open new windows, each with a node editor in?"

This kind of thing happens in real life all the time.

So in theory this should be easy. You have the FXML that defines the layout, and a controller than defines the logic associated with it. So all you need to do is have an event handler (say on a menu) that creates a new Stage and a Scene, loads the FXML again, and sets the root of the scene to the root of the FXML.

Except that it doesn't work. The problem is, you make the root pane static. So when you loaded the FMXL again, it replaced the current value of canvas with the new one from the newly-loaded FXML. If you now try to add new elements to the first canvas, you can't.

So now you have to refactor the code, so that canvas is an instance variable. Worse still, you publicly exposed it via the static getCanvas() method, which might have been called anywhere in your application. So you have to track down all those invocations. And you end up at 3am trying to grep your entire codebase to figure out what you have to change.

The problem (one problem) with making things static is that you commit to only ever having one copy of that variable, and essentially you commit to that for all time (or commit to rewriting much of the application if you change your mind). That decision leaks upwards: by only ever having one canvas, you can only ever have one controller, so you can only ever have one node editor. You have essentially prevented the code you have written from being reusable.

Your underlying assumption here is wrong. The static keyword really has nothing to do with accessibility: it is about scope. A static field is scoped to the class: a non-static field is scoped to each instance of that class. You made the field accessible by defining a public getCanvas() method and making the scope of that method - the class - available (by making the class public). If canvas is an instance variable, you can make it accessible by similarly defining a public getCanvas() method and making the scope of that method -- now the CanvasController instance -- available. So you load the FXML, retrieve the controller by calling getController() on the FXMLLoader, and pass the controller instance to whoever needs access to it. Notice now you have control in the code over who has received a reference to the controller, and if you need any refactoring you at least have a path to follow to find where the controller is used.

You mention "JavaFX doesn't have a NodeProperty or an AnchorPaneProperty: it has an ObjectProperty, so you can use an ObjectProperty<Node> or an ObjectProperty<AnchorPane> if you want/need.

In general, though, I think it's also a bad idea to expose view elements (like your canvas) outside of the controller, and even to share controllers with other parts of the application. You should instead consider using a MVC-type approach. What is missing from your question is the "M": the model. So your model would use an object model with classes like Image, Scale, Composite, etc. The classes should use JavaFX properties to make them observable and use ObservableList (and similar) to store collections of these objects and relationships among them. Create a single model instance and pass it to your controller. The controller can observe the data in the model and update the view if it changes, and update the model according to user input. Then share the model instance with the other parts of the application that need access to it. This keeps things more maintainable: if you decide, for example, canvas needs to be a Pane instead of an AnchorPane, that change is now confined to your FXML and controller, and doesn't leak out over the rest of your application. A (much simpler) example of MVC (really more MVP) in JavaFX is in this question.

You actually explained that very well, thankyou. That example future scenario you used really helped put the issue of static fields, however theres one thing I'm confused with about the MVC pattern. Using JavaFx, I'd have a `NodeController` and a `NodeModel` -- what would I instantiate? Do I have another "coupler" class `Node` that joins these two together? — haz, Jan 19 '17 at 01:14
If you are using FXML, there are a couple of different ways: either create the model, create the controller (`NodeController nc = new NodeController(nodeModel)`), create the `FXMLLoader` and set the controller "by hand" (`loader.setController(nc);`). Then get the view from the `FXMLLoader` by calling `load()`. Alternatively, set a controller factory on the `FXMLLoader`, and let the `FXMLLoader` create the controller for you. — James_D, Jan 19 '17 at 01:26
Usually I would not create a class that encapsulates that wiring, because almost all of it is handled by the FXMLLoader, but there is nothing to stop you doing that. Not sure I know that right complexity of an example for you: perhaps https://github.com/james-d/TicTacToe. Here `Game` acts as the overall model, and various components within it have their own MVC structure. (I wrote that a while ago, so it might not be perfect ;).) — James_D, Jan 19 '17 at 01:32

(JavaFx) Proper way represent static FXML fields (and why this is a bad idea)

1 Answers1