[REVIEW] Make controller instantiation customizable

[Daniel Zwolenski]

On Wed, Dec 14, 2011 at 8:33 AM, Greg Brown <greg.x.brown at oracle.com> wrote:

> > Any chance we could have the method take the string exactly as it is
> defined in the FXML, rather than have FXML create a class and pass that in?
> I'm thinking of two scenarios for this: 1) the class being loaded is via a
> different classloader (i.e. my controllers and FXML are coming from a
> repository somewhere and loaded on demand via a custom URL classloader),
>
> Two reasons why I'm not sure this is the right solution:
>
> 1) The current design is consistent with the BuilderFactory interface,
> which also takes a Class.
>
> 2) Since FXMLLoader uses the current thread context class loader, your app
> has control over how the class will be loaded.
>

That'd be pretty clunky though. I'd have to setup the classloader that
calls my FXML on the basis that the factory uses a custom classloader. i.e.
I'd be exposing the implementation details of the factory to the owner of
the factory. I'd also have to remember to unset the classloader from the
Thread when I'm done otherwise things could get messy.


> > and 2) the developer chooses to forgo tool support and instead uses a
> 'name' as the controller attribute in the FXML - this name could then be
> used to lookup the controller in a DI factory/module (this allows for
> multiple instances of the same controller class to be used, just configured
> differently, which is not supported in the proposed mechanism).
>
> Seems like it might be an edge case (i.e. falls into the "10% case" vs.
> the "90%" case").
>

All depends on how people end up using FXML. If they are using it as the
'file type' for a RAD tool then, yep, my suggestion is horribly unhelpful
and I get the feeling this has been the design-intent for FXML.

If they're using it as a declarative UI definition language as part of an
MVP pattern (i.e. the way struts uses JSPs, and the way GWT uses UIBinder)
similar to the ways that I've described in my latest blog, then 'named'
controllers will likely be preferred. I get the feeling this is not seen as
a primary use for FXML though and I'm banging it into shape to fit what I
want to do.  I think for people who have come from web land, where FXML is
conceptually similar to a JSP, they will naturally tend in the direction I
am going with it. I can't say if this will be the 10% case or the 90% case
though.



>
> > I know this is probably not inline with your view of it all, but I would
> love for a way to provide a default controller so if none was specified in
> the FXML, the loader would use this.
>
> Can you summarize a use case for that here? Controllers and FXML documents
> are pretty tightly coupled. I'm having a hard time envisioning what a
> "catch-all" default controller might do.
>

It is really a way to avoid specifying the controller at all in the FXML
file. In my pattern I have a one-to-one mapping between controller and FXML
file (like you do) but I either don't use embedded FXML or I would expect
this embedded FXML to use the parent controller (e.g. a ListCell).

If my pattern, everything is controller (aka presenter) oriented. So I
create a presenter and it comes along with its view. So I have something
like:

    Factory.getMyPresenter() {
        return (MyPresenter)loadFxmAndExtractController("MyFxml.fxml");
    }

So I have already specified the controller class in code, and now I have to
specify it again in the FXML as the controller attribute. There is zero
benefit for me to specify the controller in the FXML, and if I have a typo
(or have forgotten to set it, or change it from a previous copy-paste), I
will get a runtime class-cast error in my above code.

As I said, I'm using FXML differently to the use case I think you've
defined it for, but I do think my way of using it might be popular.



>
> > Can we have the FXMLLoader throw an exception if it tries to set a
> non-null @FXML field. That would prevent any confusion around
> double-binding when a controller is shared. I'd prefer this over the less
> obvious binding to last possible field. This would also stop a newbie error
> I see on the forum every now and then: they new their fields and then do
> stuff to it in the constructor not realising it gets created by FXMLLoader.
> An exception would sort them out sooner.
>
> Seems reasonable.
>
> > Similarly I'm not so keen on the null resulting in default behaviour as
> it could lead to confusion as well.
>
> This behavior is also consistent with BuilderFactory, so I think that
> defining a different behavior for ControllerFactory might actually be more
> confusing.
>
>
I'll just add a null check to my code and throw an exception then. I can
see some poor developer not doing this though, and spending frustrating
hours trying to work out why when they update their 'shared' bean it
doesn't reflect in their GUI - only to find out hours later that it is not
actually 'shared' because the FXMLLoader magically created a new instance
without really being asked to (or only inadvertently being asked for). As I
said, I'll live.