Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Injected vs. GuiceFXMLLoader controller contract #20

Open
paxbit opened this issue Feb 21, 2015 · 1 comment
Open

Injected vs. GuiceFXMLLoader controller contract #20

paxbit opened this issue Feb 21, 2015 · 1 comment

Comments

@paxbit
Copy link

paxbit commented Feb 21, 2015

Hi headcr4sh,

I'm using fx-guice for a day now. I found it easy to set up but I stumbled upon a behavior I'm not sure I fully understand so far but which would probably be a deal breaker for my current project at hand if it turns out the way I think it is. I'd appreciate it if you could help me understand it.

Is it true and intended that a @FXMLComponent controller always has to be its own root node (extends Node) and that the FXML of such a controller must use an <fx:root> without specifying an fx:controller, because otherwise a javafx.fxml.LoadException: Root value already specified. is thrown? And is it therefore true that FXMLs using an actual Node (like <StackPane ...>) as root and/or specifying a fx:controller attribute can only be loaded using GuiceFXMLLoader, hence never be @Injected?

As far as I can see the former cannot be any different since FXMLComponentMembersInjector supplies the controller instance to both FXMLLoader.setController and FXMLLoader.setRoot in FXMLComponentMembersInjector.injectMembers(...). I'm aware that using <fx:root> along with a component controller of Node is a common way to build reusable components but I'm missing the ability to also inject controllers not following this pattern, which are not a Node and whose FXML root node is not <fx:root> but an actual Node with a fx:controller attribute. Do you think you could implement that, are you interested in doing it or do you think it cannot or should not be done? If so, could you please elaborate why?

My last question targets the exception handling. Do you see any way to actually handle a FXMLLoader specific Exception thrown while injecting (see the Runnable created in FXMLComponentMembersInjector.injectMembers(...)) in user code? Right now, if something goes wrong, you end up with a half initialized injected controller instance with all @FXML fields unbound and still null and without any way to react to the failure. I think it would be very desirable to be able to handle those Exceptions gracefully.
I don't know why but setting a Thread.currentThread().setUncaughtExceptionHandler(...) for the Application Thread did not work out of the box. But that would be a crutch anyway.

Edit: Forgot sth. yesterday. About the limitation, which somewhat impairs productivity, the FXML may not declare a 'fx:controller' when used in conjunction with @Inject because this would cause a javafx.fxml.LoadException: Controller value already specified.: I think using fxmlLoader.setControllerFactory(clazz -> instance) instead of fxmlLoader.setController(instance) in FXMLComponentMembersInjector.injectMembers(...) will safely get around that issue while still allowing allowing IDEs to infer the controller of an FXML. So all the nice inspections and code completions work in the controller and the FXML. Also, SceneBuilder can see the controller.

Thanks for your attention, appreciate your reply, regards,
paxbit

@headcr4sh
Copy link
Member

Hi paxbit,

Is it true and intended that a @FXMLComponent controller always has to be its own root node (extends Node) and that the FXML of such a controller must use an fx:root without specifying an fx:controller, because otherwise a javafx.fxml.LoadException: Root value already specified. is thrown? And is it therefore true that FXMLs using an actual Node (like <StackPane ...>) as root and/or specifying a fx:controller attribute can only be loaded using GuiceFXMLLoader, hence never be @injected?

The @FXMLComponent annotation is indeed ment for components. There is a second annotation, though -- @FXMLController -- which might suit you need.

As far as I can see the former cannot be any different since FXMLComponentMembersInjector supplies the controller instance to both FXMLLoader.setController and FXMLLoader.setRoot in FXMLComponentMembersInjector.injectMembers(...). I'm aware that using fx:root along with a component controller of Node is a common way to build reusable components but I'm missing the ability to also inject controllers not following this pattern, which are not a Node and whose FXML root node is not fx:root but an actual Node with a fx:controller attribute. Do you think you could implement that, are you interested in doing it or do you think it cannot or should not be done? If so, could you please elaborate why?

Not sure what you mean,... maybe another situation where @FXMLController would be a better fit?

My last question targets the exception handling. Do you see any way to actually handle a FXMLLoader specific Exception thrown while injecting (see the Runnable created in FXMLComponentMembersInjector.injectMembers(...)) in user code? Right now, if something goes wrong, you end up with a half initialized injected controller instance with all @FXML fields unbound and still null and without any way to react to the failure. I think it would be very desirable to be able to handle those Exceptions gracefully.
I don't know why but setting a Thread.currentThread().setUncaughtExceptionHandler(...) for the Application Thread did not work out of the box. But that would be a crutch anyway.
Mh... exception handling is something that should indeed be improved.

I don't think that a global exception handler would be a very smart idea,... maybe (I'll have to think about that a bit longer) it might be useful to have a method that may (or may not) be overwritten in the GuiceApplication class.

About the limitation, which somewhat impairs productivity, the FXML may not declare a 'fx:controller' when used in conjunction with @Inject because this would cause a javafx.fxml.LoadException: Controller value already specified.: I think using fxmlLoader.setControllerFactory(clazz -> instance) instead of fxmlLoader.setController(instance) in FXMLComponentMembersInjector.injectMembers(...) will safely get around that issue while still allowing allowing IDEs to infer the controller of an FXML. So all the nice inspections and code completions work in the controller and the FXML. Also, SceneBuilder can see the controller.

Whoops.
Have to check my code. Settings the controller factory instead of the controller is of course the way to go...

I'm willing to accept pull-requests to the fx-guice codebase as long as they don't break existing functionality. So... feel free to propose your changes. ;-)

-Benjamin-

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants