Re: Add a "ControllerBinder" in UiBinder so that UiHandlers can be bound on both the (issue923801)
rj...@google.com
My team has been struggling a bit to find a way to handle
interactions between controllers and views smoothly. We've ended up
with a lot of code in one of two styles:
class FooController {
void onFirstRender {
view.addClickHandlerToSomeButton(new ClickHandler()
{ handleEvent(ClickEvent e) { /* ... */ } });
}
}
or
class FooView {
private FooController controller;
@UiHandler("fooElement")
void handleClick(ClickEvent e) {
controller.doSomething();
}
}
We do not love either of these styles -- the first one is very
cumbersome, since we have to add methods to the view, and the second
one is nasty because it makes it easy for business logic to leak into
the view. We really don't want our views to store references to their
controllers.
After hashing this out with a few devs, we came up with a compromise
that we like:
class FooController {
@UiHandler("someElementInView")
public void handleClick(ClickEvent e) {
doSomeControllerLogic();
}
}
class FooView {
@UiHandler("someOtherElement")
public void handleClick(ClickEvent e) {
doViewSpecificLogicOnly();
}
}
This requires a little bit of modification to UiBinder. I have a hacky
CL that allows this, by letting the view use a "ControlledUiBinder"
rather than a UiBinder that points the code generator to the
controller and adds hooks for those handlers, then putting a UiBinder
on the controller that places those handlers on the view's binder.
Before I turn this into a 'ready-for-prime-time' CL, does this sound
like a reasonable change? The existing UiBinder syntax remains the
same, but there are two new interfaces extending it that can be used
instead. And there's no more calls to setController(), circular
generics, or business logic in the view (all problems my team has
had).
I can go into more detail if people are interested. I just wanted to
float an idea balloon.
rj...@google.com
problem best: allow the view to have knowledge of its controller, but in
its own terms, via a delegate interface that the view defines. The
little bit of code you wind up with in most views becomes a semantic
passthrough to the delegate.
interface FooView {
interface Delegate {
void saveClicked();
void cancelClicked();
}
BarView getNestedBarView();
}
A click event on the Save button becomes a call to
delegate.saveClicked(), basically translating browser events to semantic
method calls. Dan Danilatos covered it in detail here:
http://code.google.com/events/io/2010/sessions/gwt-continuous-build-testing.html
Back to the patch, if I read it right you're making the fields of a
uibinder owner define its public API, which seems like a pretty bad
thing to encourage. To avoid leaking business logic into your views
you're flooding your controller with knowledge of its view
implementation. How do you test these things? How does the next guy know
whether his event handler belongs in the view or the controller?
I agree that we need a more general low-labor way to bind events in a
GWT app, but this doesn't seem to be it. Am I missing something?
Stephen Haberman
> I agree that we need a more general low-labor way to bind events in a
> GWT app, but this doesn't seem to be it. Am I missing something?
It's not necessarily related to events, but I've been pleased with the
low-labor aspect of generating views:
http://gwtmpv.org/viewgeneration.html
My workflow:
* In Foo.ui.xml, add a "ui:field=name" line
* Eclipse project builder sees the change, automatically runs codegen
* IsFooView is updated with "IsTextBox name()"
* GwtFooView is updating with @UiField name/etc.
* StubFooView is updated with "StubTextBox name()"
* In FooPresenter, which extends Presenter<IsFooView>, add:
* view.name().addClickHandler(...)
Given GwtFooView is entirely generated, I cannot use @UiHandler events,
hence FooPresenter calling "addClickHandler".
However, I don't find this to be a big deal, as I'm trending towards a
gwt-pectin-style binding DSL:
// in FooPresenter onBind
binder.bind(model.nameProperty).to(view.nameTextBox)
So still no inner-classes, and the model/view keep each other up to date
when one or the other changes. Note that the model part is entirely
optional, it's only required if you want the handy "binder.bind" syntax.
- Stephen
Jeff Larsen
-----
whether his event handler belongs in the view or the controller?
If the event doesn't change the view state, it doesn't belong in the
view. I would argue that clicking a save button doesn't change the
view, so it shouldn't be added to the view, but clicking on the header
of a disclosure panel does change the view state, so it belongs in the
view.
As far as how you test them, by coding in the presenter specifically
against interfaces, you can use stubs to run your tests, still getting
all the JUnit testable goodness,
Don't forget, that by mapping each widget to an interface would allow
you to bind those up with GIN and create providers for those inside
the presenter, so if for some reasons you wanted to be able to alter
the view, you could easily do it and still keep the presenter/activity
JUnit clean.
A first awesome step to this would be to make every widget implement
an interface exposing all their public methods. I really hated
extending every control and it will become more and more of a
nightmare to maintain as the gwt codebase changes.
I'd like to look more at the ui:binder changes. My idea was to add an
annotation/annoatation processor to automatically codegen a new
interface of every public method. For example
@GenerateDisplayInterface
class SomeView implements SomeViewDisplay{
@UiField
TextBox box;
@UiField
Label label;
public SomeView(){.....}
public IsTextBox getBox(){
return box;
}
@DoNotGenerate
public IsLabel getLabel(){
return label
}
}
would generate
public interface SomeViewDisplay {
IsTextBox getBox();
}
This would then avoid the problems inhernate with having the view be
completely code genned and would allow me to put view state change
handlers inside the view.
I would LOVE LOVE LOVE for every gwt object that extends UIObject to
implement a specific interface and would be more than happy to
implement this.
On Oct 19, 1:35 pm, Stephen Haberman <stephen.haber...@gmail.com>
wrote:
Stephen Haberman
> How do you test these things?
This "gwt-hack" example project is horribly unmaintained, but the idea is:
When GwtFooView is generated, so is StubFooView, which has fields like:
StubTextBox name;
public IsTextBox name() { return name; }
So the test can grab the StubTextBox and call methods on it, like
"click" or "type" or "focus", which fire dummy versions of ClickEvent,
ChangeEvent, etc. The presenter handles these just as it would native
events (well, as long as you're not using native event features), and
then the test can observe the side effects in the stub widgets. E.g.
that the textbox's getText changed, or had style=display:none set, or
what not.
Here is StubTextBoxBase, for example:
I have admittedly added stub functionality in an ad hoc manner, so I
can't promise that it acts exactly like the browser does, but the idea is to
get close enough that it's good enough for unit testing.
> How does the next guy know whether his event handler belongs in the
> view or the controller?
Well, the view is generated, so any code he puts there will be
overwritten. :-)
> [snip] but clicking on the header of a disclosure panel does change
> the view state, so it belongs in the view.
Personally, I like putting everything in the presenter so it stays
testable.
> Don't forget, that by mapping each widget to an interface would allow
> you to bind those up with GIN and create providers for those inside
> the presenter, so if for some reasons you wanted to be able to alter
> the view, you could easily do it and still keep the presenter/activity
> JUnit clean.
Yep. :-) I don't use GIN, but I have a WidgetsProvider interface that
allows the presenter to create dynamically create "newTextBox" widgets
and have it be a stub/gwt version as appropriate.
> A first awesome step to this would be to make every widget implement
> an interface exposing all their public methods. I really hated
> extending every control and it will become more and more of a
> nightmare to maintain as the gwt codebase changes.
Agreed. I've been doing this lazily, as my apps need things:
http://github.com/stephenh/gwt-mpv/tree/master/user/src/main/java/org/gwtmpv/widgets
> I'd like to look more at the ui:binder changes. My idea was to add an
> annotation/annoatation processor to automatically codegen a new
> interface of every public method. For example
:-)
I started out doing this:
http://github.com/stephenh/interfacegen
But then I wanted stubs too:
http://github.com/stephenh/stubgen
And finally I gave up on APT (for this problem), because I don't want to
have to code the SomeView class at all. All of the XxxView artifacts are
derivable from the `ui.xml` file, so why maintain any of it by hand?
Given that the APT API only supports Java classes, I had to fail over to
a standard Java program that parses all of the `ui.xml` files in a
project.
However, given custom builders in Eclipse (which I need to do a write up
of), I've found this approach to be just as nice (it runs automatically
on save) as APT.
> I would LOVE LOVE LOVE for every gwt object that extends UIObject to
> implement a specific interface and would be more than happy to
> implement this.
Me too. :-)
I'd love to have help maintaining what I've got started. Andrew from
gwt-pectin has expressed interest in them as well--either from within
gwtmpv or extracting them into a gwt-widget-interfaces project.
Obviously it would be best if they were in GWT proper, but I haven't
seen any indication that it is likely to happen.
- Stephen
Ray Ryan
On Tue, Oct 19, 2010 at 10:16 AM, <rj...@google.com> wrote:
interface FooView {
interface Delegate {
void saveClicked();
void cancelClicked();
}
BarView getNestedBarView();
}
Yeah, we've taken that approach on various occasions, but I don't think that achieves the appropriate separation of views and controllers. In the idealized MVC world, I don't want to have to know anything about how my controller works -- extending even to its API. The Delegate method always seemed like a kludge to me, particularly since our team injects both views and controllers pretty aggressively -- meaning the view still needs to know the full type of its controller in the constructor for us to @Inject it -- or we have to manually write an @Provides method for every V/C pair with event handlers.
Back to the patch, if I read it right you're making the fields of a
uibinder owner define its public API, which seems like a pretty bad
thing to encourage.
I don't expose an API on the view -- the names of the @UiField-annotated objects are the only thing exposed to the controller. As I understand it, they already are exposed to allow UiBinder to reflect over the view class.
To avoid leaking business logic into your views
you're flooding your controller with knowledge of its view
implementation. How do you test these things? How does the next guy know
whether his event handler belongs in the view or the controller?
View event handlers are those that don't need to interact with the controller (this checkbox triggers that checkbox, etc.). Controller handlers are the methods that would be defined in the Delegate interface above.
The testing protocol for these functions is the same as what we have now. In the current scheme, you test the methods specified by Delegate on the controller -- it would still be this way. They just aren't defined by an interface anymore.I'm not sure what you mean when you say the controller is "flood[ed] with knowledge of the view implementation"
Having methods on the controller named saveClicked() and cancelClicked() is not very different from having @UiHandler("save") -- you're encoding the same information with different semantics -- except with my patch there's less code and less duplication, since you can skip both the Delegate interface and this inevitable method:
@UiHandler("save")void handleSaveEvent(ClickEvent event) {
delegate.saveClicked()}instead, you just add this to the controller:@UiHandler("save")void saveClicked(ClickEvent event) {
/*... business logic ...*/
}
I agree that we need a more general low-labor way to bind events in a
GWT app, but this doesn't seem to be it. Am I missing something?
The current approach produces a lot of boilerplate for us. Moreso, once the Delegate is in the view, it's much more tempting for future maintainers to let business logic seep into the view (we've seen this repeated over and over). Once a few controller methods are exposed in an interface, the rest often come tumbling in, despite our best efforts.
- doesn't it require the existence of a ui.xml file? That's a bad requirement for a controller.
- your controllers now rely upon a binding object generated by GWT. Didn't you just close the door on JRE tests for them?