FXML Patterns / best practices (original) (raw)
Daniel Zwolenski zonski at googlemail.com
Thu Feb 23 20:58:25 PST 2012
- Previous message: FXML Patterns / best practices
- Next message: FXML Patterns / best practices
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hey guys,
This is a good point to work from, nice one.
You have both gone for the approach of the FXML 'Controller' being part of the V of the MVP or MVC pattern. So we have really M(F+C)P or M(F+C)C patterns - where (F+C) is an FXML file and FXMLController combo making up the View. In this approach we've got our classic 'Presenter' or 'Controller' as well as the FXMLController. There's a few variations in how each of you implemented this but the high-level result is pretty similar.
There's a few minor tweaks I would make to Greg's code (such as moving the FXML loading out of the Presenter and into a factory) and there's probably merit in incorporating a few elements from Michael's implementation too. That aside, yes, the pattern has been achieved with the original benefits. There's a naming clash, in that 'Controller' has a definite role in MVC and a conceptual one in MVP, yet we've now got another 'controller' inside the View - but for me at least, I'm happy to sidestep that issue for now.
One question for both of you (and all of you): Is this what you guys would consider the recommended/best-practice for implementing this? If you had to implement this sort of functionality (i.e. retrieve some data from a remote service, show it, allow it to be edited, using FXML etc) in your own application is this what you would use. If not, what, in your mind, is the best architecture for implementing this?
Looking at this proposed implementation, we've now got FXML in there for building up our Scene Graph, which is fantastic but the need for this extra FXMLController is a new cost. It is an extra file (which I have no problem with so long as it adds value), and also means we still have Java files within the designer's domain (i.e. the plumbing of our View).
My question: is this cost one we have to pay, or is there a way to avoid it (by potentially tweaking FXML)?
Looking at Greg's FXMLController, it looks like this:
public class PersonViewController extends Controller { private ObjectProperty presenterProperty = new SimpleObjectProperty(); private ObjectProperty modelProperty = new SimpleObjectProperty(new PersonModel());
... property accessors ...
@FXML protected void handleDeleteButtonAction() {
getPresenter().delete();
}
@FXML protected void handleSaveButtonAction() {
getPresenter().save();
}
}
So basically it holds onto a couple of variables and just routes methods back to our Presenter but it doesn't actually add any real value (or does it? if it does add value, I could be convinced it's worth having?).
In the context of this pattern, would it not be better if we could just pass those properties (i.e. the model and presenter) straight into the FXML file and dodge the need for this FXMLController. i.e. our FXML would look something like:
<VBox xmlns:fx="http://javafx.com/fxml">
<Button text="Delete" onAction="#*presenter.handleDeleteButtonAction*"/>
<Button text="Save" onAction="#*presenter.**handleSaveButtonAction*"/>
So the above FXML, instead of assuming there is a PersonViewController is just assuming there is a variable in its name space called 'presenter' and another called 'model'. The thing loading this FXML is responsible for providing these. For me this seems a lot better as we've avoided the PersonViewController, resulting in one less Java class and implementing our View 100% in FXML. Are there reasons anyone can see for not supporting this in FXML? (I'm also aware we can achieve something sort of like the above using JScript and some mucking around, but that's messy and not really 'supporting' it, in my opinion).
One drawback for me is that we've lost the 'type' that was defined by the fx:controller declaration before, which means IDE/tools can't help us if we misspell properties or methods, or they don't exist, etc. If this was important (which I think it is) we could add tags at the top of the file somewhere to allow these types to be defined, e.g.
This is very similar in concept to defining fx:controller="examples.fxml.presenter.PersonPresenter" only more flexible as anything could be used to handle callbacks, whereas in the fx:controller option, it has to be the "controller". As a side benefit, we also resolve our whole naming clash issue, as I can call it a controller or a presenter or a handler or whatever I like.
This approach also fully allows for the current FXML design pattern, i.e. a non-MVP one where a single view-controller is used as a 'backing bean' and there is no actual model. You would just declare the 'controller' as a param instead of using the fx:controller attribute. So no one is hurt by this change, and the MVP pattern benefits.
That's one suggestion, and perhaps it has flaws? I'm sure there are also other ways of removing the redundant PersonViewController class (or hearing reasons why it is not actually redundant). I'm definitely open to suggestions.
This is more or less the spot we hit last December however, and the conversation then just died-out. So my underlying question is, is there interest in improving FXML in order to support MVP (and other patterns) or is it just not something you guys see as important and what we've got is what we've got? I don't want to be causing trouble and distracting from other tasks if this is not important, but if it is important, I think together we can work out ways to improve things. I do think however that adding more stuff around the FXML Controller class (e.g. an abstract base class for it) has a risk of locking us into it and potentially ruling alternatives out, so in my mind it's worth resolving this question first.
Cheers, Dan
On Fri, Feb 24, 2012 at 1:29 AM, Greg Brown <greg.x.brown at oracle.com> wrote:
Here's a working example that demonstrates one way this could be implemented using FXML. It won't work on 2.1 since it uses bi-directional binding and the controller enhancements I have proposed, but you should be able to get a good idea for how it works from the source code.
The attached archive includes the following files: - PersonModel.java - person model (complete implementation of your example PersonModel class) - personview.fxml - markup for person view - PersonViewController.java - controller for person view - PersonPresenter.java - presenter implementation that uses PersonModel and personview.fxml - Gender.java - enum definition for gender - PresenterExample.java - sample launcher application A screen shot of the running application is also attached. Note that everything works as expected with one exception - since ChoiceBox does not define a "selectedItem" property, it was not possible to declaratively create the binding to the "controller.model.gender" property. Hopefully this will be resolved in a future release. Let me know if you have any questions. Greg
On Feb 23, 2012, at 3:05 AM, Daniel Zwolenski wrote: > I'm spawning off a new thread for this, so Greg's discussion on the > Controller base class can continue uninterrupted. > > I just wrote a huge email outlining all the pros/cons of clean MVP and > how/where I think FXML is causing me to compromise, etc. It was too long > and too complicated, so back to basics. Below I define a very pure MVP > implementation (in psuedo code) of a Person Component using pure Java. I'll > avoid defining interfaces at this point for ease, but ideally I would do > this as well (so I can use mock objects when testing). I've used a > standard, common, popular pattern that I believe a lot of people would like > to use if they could. > > What I'd like to know is how people intend to implement something like the > below using FXML. I'm looking for either the closest approximation of the > below using FXML, or if you think you have a better pattern, then how you > would implement this. Based on that I'd then like to look at what > advantages/disadvantages your proposed design has over the 'pure' MVP > approach below (or tell me what you'd do differently if you don't think the > below is pure MVP!). > > The key benefits that I see from the below architecture are: > > 1. Seperation of concerns: the relevant implementation details are very > well contained within each of the correct elements, view in View, > presentation logic in Presenter, model stuff in Model. > 2. Unit Testability: the well-defined units are easily decoupled from > each other and can be tested in isolation allowing for clean unit tests > 3. Reusability: each element of the MVP bundle is reusable, so you could > attach a different View (e.g. a mobile specific one) to the Presenter, etc > > Can we retain these benefits in an FXML-based architecture, or do we have > to give them up? > > > Model > > class PersonModel { > > private LongProperty personId; > private TextProperty firstName; > private TextProperty lastName; > private ObjectProperty gender; > > ... constructor, getters, setters ... > } > > View > > class PersonView extends VBox { > > @Inject private PersonModel model; > @Inject private PersonPresenter presenter; > > private TextField firstNameField; > private TextField lastNameField; > private ChoiceBox genderField; > private Button deleteButton; > private Button saveButton; > > @PostConstruct void init() { > > firstNameField = new TextField(); > > firstNameField.textProperty().bindBidirectional(model.firstNameProperty()); > getChildren().add(firstNameField); > > lastNameField = new TextField(); > > lastNameField.textProperty().bindBidirectional(model.lastNameProperty()); > getChildren().add(lastNameField); > > genderField= new ChoiceBox(Gender.values()); > > genderField.selectedValueProperty().bindBidirectional(model.genderField()); > getChildren().add(genderIcon); > > deleteButton= new Button("Delete"); > deleteButton.setOnAction(new EventHandler() { > public void handle(ActionEvent event) { > presenter.delete(); > } > }); > getChildren().add(saveButton); > > saveButton= new Button("Save"); > saveButton.setOnAction(new EventHandler() { > public void handle(ActionEvent event) { > presenter.save(); > } > }); > getChildren().add(saveButton); > } > } > > Presenter > > class PersonPresenter { > > @Inject private PersonModel model; > @Inject private PersonView view; > @Inject private PersonService personService; // e.g. an RMI-like > handler onto a server > > // this is the external method called by whatever wants to show a person > void showPerson(final long personId) { > new Thread(new Task() { > PersonDTO call() { > return personService.getPersonDetails(personId); > } > > void onSuccess(PersonDTO details) { > model.setPersonId(details.getPersonId()); > model.setFirstName(details.getFirstName()); > model.setLastName(details.getLastName()); > model.setGender(details.getGender()); > } > }).run(); > } > > void delete() { > final long personId = model.getPersonId(); > new Thread(new Task() { > void call() { > personService.deletePerson(personId); > return null; > } > > void onSuccess(Void result) { > // probably navigate away from the current view > } > }).run(); > } > > void save() { > final PersonDTO details = new PersonDTO( > model.getPersonId(), > model.getFirstName(), > model.getLastName() > model.getGender() > ); > > new Thread(new Task() { > void call() { > return personService.updatePerson(details); > } > > void onSuccess(Void result) { > // probably show a nice confirmation message > } > }).run(); > } > } > > > The above assumes the existence of some sort of factory method that wires > everything up. This would ideally be Spring or Guice but could just as > easily be done by hand like so: > > public PersonPresenter createPersonPresenter() { > > PersonModel model = new PersonModel(); > PersonView view = new PersonView(); > PersonPresenter presenter = new PersonPresenter(); > > view.setModel(model); > view.setPresenter(presenter); > > presenter.setModel(model); > presenter.setView(view); > presenter.setPersonService(getPersonService()); > > return presenter; > }
- Previous message: FXML Patterns / best practices
- Next message: FXML Patterns / best practices
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]