A few comments
On 27 juin, 13:53, Paul Schwarz <paulsschw...@gmail.com> wrote:
> When using the EventBus, for each event type in your system you will
> create a Class (a GwtEvent) and an Interface (the corresponding
> EventHandler).
>
> It is a bit of a nuisance maintaining two java files for each event.
Not necessarily 2 files. As you show below, it's becoming common usage
to declare the handler as an inner/nested interface of the event
class.
> So I propose to simplify it by having one abstract event class and
> then ONLY ONE class for each event, instead of two. Note that your
> actual usage of your new style event class stays the same, so there is
> no refactoring required there.
That's not totally accurate, see below.
> ___________________________________________
> 1.
> In your com.company.project.shared package create this file:
>
> import com.google.gwt.event.shared.EventHandler;
> import com.google.gwt.event.shared.GwtEvent;
>
> public abstract class AbstractEvent
> <E extends GwtEvent<H>, H extends AbstractEvent.AbstractHandler<E>>
> extends GwtEvent<H> {
>
> public static interface AbstractHandler<E> extends EventHandler {
> void handleEvent(E event);
> }
The problem with such a generic interface is that you can't implement
2 of them on the same class. For instance, in my presenters I
generally create an inner Handlers class implementing all event
handler interfaces I need:
class MyPresenter {
private class Handlers implements
PlaceChangeRequestedEvent.Handler, PlaceChangeEvent.Handler,
EmployeeRecordChanged.Handler {
public void onPlaceChange(PlaceChangeEvent event) { ... }
public void onPlaceChangeRequested(PlaceChangedRequestedEvent
event) { ... }
public void onEmployeeChanged(EmployeeRecordChanged event)
{ ... }
}
@Inject
public MyPresenter(HandlerManager eventBus) {
Handlers handlers = new Handlers();
eventBus.addHandler(PlaceChangeRequestedEvent.TYPE, handlers);
eventBus.addHandler(PlaceChangeEvent.TYPE, handlers);
eventBus.addHandler(EmployeeRecordChanged.TYPE, handlers);
}
}
Using a generic handleEvent method makes this impossible. Well, not
impossible, but cumbersome, as you have to do some if/else with
instanceof in your handler:
public void handleEvent(AbstractEvent event) {
if (event instanceof CalendarChangeRequestEvent) {
CalendarChangeRequestEvent ccre =
(CalendarChangeRequestEvent) event;
...
} else if (event instanceof EmployeeRecordChange) {
EmployeeRecordChange erc = (EmployeeRecordChange) event;
...
}
}
I'm not saying this is a show blocker, but it can then imply some
refactoring, which is not what you "promised" above ;-)
I'm not saying this is a show blocker, but I nonetheless suspect it
could be an issue, otherwise GwtEvent would have gone this way from
the beginning, or the RecordChangedEvent recently introduced.
I know some people complained about not being able to implement two
ValueChangeHandler on the same class (in this case you'd check the
event's source to "disambiguate" the events).
Personally I'm using a single inner class implementing all handlers
instead of many anonymous handler classes, because I know a class has
a cost in the output JavaScript cost. I can't tell "how many" it costs
and whether the cost is negligible, and I know it costs less in each
GWT release due to compiler optimizations (-XdisableClassMetadata, GWT
2.1 introduces some clinit pruning AFAICT), but still, it doesn't make
my code less readable (not particularly more readable either) and
implies only one class initialization and instantiation instead of, in
the above example code, three.
See for instance:
http://code.google.com/p/google-web-toolkit/wiki/ClassSetupAndInstantiationOptimization
http://code.google.com/p/google-web-toolkit/wiki/AggressiveClinitOptimization
http://code.google.com/p/google-web-toolkit/wiki/ClinitOptimization
All in all, I don't think it's really worth it, you're only saving 6
lines of code (3 for the handler declaration, and 3 for the
dispatchEvent implementation) with no substantial gain as a result.
> @SuppressWarnings("unchecked")
> @Override
> protected void dispatch(H handler) {
> handler.handleEvent((E) this);
> }
>
> }
>
> ___________________________________________
> 2.
> This is what an actual event class will look like. I think you'll
> agree that this is much simpler than before. Notice we've even got rid
> of getter methods for the attributes and replaced them with public
> final fields instead; this is because we're never changing the event
> data, so it CAN be declared public final, AND that reduces the bulk of
> the class too:
Again, you're only saving a few lines of code (which an IDE would
generate and maintain for you) without substantial gain (getters would
be inlined anyway by the GWT compiler), but breaking the common Java
coding practice (or more broadly OOP pattern) of encapsulation.
I'd add, for newcomers, that the fact that you declared fields "final"
has nothing to do with your AbstractEvent.
--
You received this message because you are subscribed to the Google Groups
"Google Web Toolkit" group.
To post to this group, send email to google-web-tool...@googlegroups.com.
To unsubscribe from this group, send email to
google-web-toolkit+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/google-web-toolkit?hl=en.
