[REVIEW] Make controller instantiation customizable

[Daniel Zwolenski]

On Thu, Dec 15, 2011 at 8:25 AM, Greg Brown <greg.x.brown at oracle.com> wrote:

> > What about one of the following in the FXML (your choice which one);
> >
> > <HBox fx:controller>
> > <HBox fx:controller="">
> > <HBox fx:controller="default">
> > <HBox fx:default-controller>
>
> FYI, I don't believe the first and last options are valid XML, so I think
> those are out in any case.
>

Good point. My bad.


>
> But I guess I'm still not clear on what you mean by "default". I
> understood the idea of providing a controller to the loader rather than
> having the loader instantiate one, but I don't think I could describe what
> fx:controller="default" means.


I am using default to mean 'untyped' (see next section below). What I am
saying in the FXML is "I don't want to specify the controller instance in
the FXML, just give me whatever the loader thinks the controller should be
and I will bind to that".

I think the problem you are facing is what to do in the case when no
factory is used and you need to try and instantiate a controller that is
untyped (i.e. has a 'class' of 'default' in my above scenario). I agree,
it's a problem - probably the only solution is to throw an FXMLLoad
exception, much as you would if the class specified didn't exist - i.e. by
default the FXMLLoader can't handle this special case, it requires a custom
factory.

As I said when I first raised this one - it is not inline with your current
thinking/approach. You have designed everything around the FXMLLoader
creating classes, I specifically am trying to get around this. I'm looking
for a compromise where your stuff all still works and the back door is left
open for me to do my stuff.

Making the factory take a String instead of a class would do this for me,
and you wouldn't have to put in a default controller. This whole
question/problem would just disappear for you as it would just be a hack I
do in my code. The use of 'Class' as a parameter instead of 'String' has
closed the door on my hack.


> > 1. The FXML declares named/typed variables that the loader can supply.
>
> FXML isn't typed, so this doesn't make a lot of sense to me.


FXML is not 'compiled' but it is most definitely 'typed'. It explicitly
defines a type of controller in fx:controller, which both the RAD tool
(from what I know) and the runtime then validate against (in lieu of the
compiler doing it). IDEs will likely also validate against this type when
they get round to having FXML support.


> But as we have already discussed, you can pass values to the loader via
> the namespace, even before you call the load() method.
>

Yes but the namespace *is* untyped. There is nothing in the FXML that tells
the tooling what type each of the namespace variable are, the tool cannot
validate this for example:

<Label text=${myNameSpaceVariable.someTextValue}/>


So if we start using Namespaces for this sort of stuff we lose tooling
support, which is not such a big deal for me, but is a major concern of
FXML (seems to be its main driving force), and for a lot of other people
from this conversation, including yourself and Richard I believe. It would
be fair to say that use of the namespace is not going to be well supported
by tools (they will have to just ignore it).

Aside from the tooling issue this namespace thing won't work for me in its
current form anyway. Some things I can hack around like turning on
JavaScript support to handle method callbacks:

<Button onAction=${controller.soSomething}/>


But I don't believe I can access the event if I want it, using this
approach? In the case of mouse events, I need the event to know where the
click was:

<ImageView onMouseClicked=${controller.handleMouseClick}/>
// handle mouse click won't know click location


I also can't actually link an actual field back to a namespace variable
either (i.e. what we normally use the fx:id for) so a controller is still
required if I want to access the field, and if I need a controller there's
no point bothering with the namespace and we're back to square one.

Bi-directional binding (assuming it works on namespace variables?) will
reduce the need to access the fields directly (i.e. I can bind to the
properties of the field) but there are still cases where I would like
access to the field directly such as to set a list CellFactory or access
the lists selectionModel.

What I'd need is something like this:

<Button fx:assignTo=${myNameSpaceVariable.myButton}/>


Which would assign the Button to the bean property called myButton on a
variable defined in the namespace with the name 'myNameSpaceButton'.

So yes, the namespace solution is the start of what I am suggesting, but
there is more needed to make it work. Hence all this discussion :)


> > 2. The FXML 'implements' a view interface. Tooling would generate the
> interface instead of the controller class.
>
> Again, FXML isn't typed, so it can't really "implement" anything. Also
> note that the current set of tooling doesn't actually get involved in any
> code generation, only markup. That's one of the benefits of the FXML
> approach - it avoids the messiness typically associated with GUI
> builder-generated code.
>

I admit, I'm surprised by this. So the current tool doesn't create a
Controller class and automatically put @FXML fields on it for things I
define in fx:id in my FXML? What about auto generating methods that are
defined in onAction="#doSomething".

Sounds more like a Scene Builder (i.e. draw some layouts) than a
RAD tool (rapid application development - a bit more end-to-end) then? If
its just a Scene Builder why does it need to know about the controller at
all then? Does the RAD tool use the fx:controller attribute for anything?


>
> I'm really struggling to understand the use case.


Have you read this:
http://www.zenjava.com/2011/12/11/javafx-and-mvp-a-smorgasbord-of-design-patterns/

I want to achieve a pure MVP pattern without the compromises outlined in
option #4 (the FXML comprompise).

Check out option #2: interfaces for the purists. In a perfect world this
pattern could be implemented with FXML being a hot-swap for the view impl
(e.g. 'ContactViewImpl'). Currently this is not possible, see option #4.
Actually in a perfect world, FXML could be used in any of the patterns and
many more with no changes needed outside of the 'view' class - this is the
goal of encapsulation and decoupling. This is not currently achievable.



> Couldn't you do something like this, for example?


> public interface MyController {
>    ...
> }
>
> public class MyControllerImpl implements MyController {
>    ...
> }
>
> <HBox fx:controller="com.foo.MyController">
> …
> </HBox>
>
> Your controller factory's implementation of getController() could return
> an instance of MyControllerImpl or whatever implementation makes sense.
>

On a technical front this won't work at the moment because I don't think I
can define an @FXML field on an interface that an fx:id would map to. I
don't believe @FXML will map back to a method for this and interfaces can't
define fields?

Also, note that this is going to break your FXMLLoader if a factory is not
defined, since it can't now instantiate the controller (since it is an
interface). So from your point of view I would have thought this is just as
bad as the 'default' controller defined above. It won't run without a
factory, and if the RAD tool is trying to instantiate controllers to
preview the GUI (not sure if this is what it does or not) then it will fail
too.

Otherwise yep, this is exactly the same as the named variable proposal,
except your suggestion is for only one variable (i.e. the 'controller'
variable). I'm just proposing that we extend this beyond the controller to
the namespace and downgrade the controller to be just another variable in
the namespace. Puts the power back in the hands of the developer to use
whatever pattern they want (models, multiple controllers, etc).

So to achieve the above you would do this:

<param name="controller" type="com.foo.MyController>


And then in your code that uses it:

<Button onAction=${controller.doAction}/>