Thanks for the crits Thomas, I've learned something from this: keeping handlers together in a given class. That's quite neat because really what I was driving at is not so much optimising output of the GWT compiler, but more cutting down on the cognitive load on the developer and time it takes (just a minute, but still) to have to create a separate Event Class and Handler Interface for each event type, and then what about refactoring? If you use my method (ok, we lose your very nice "composite" nature) you will find that Eclipse's code completion totally gets what you're trying to do when you copy/paste to create a new event; with the Event+Handler pair you'd have to first copy and rename your interface, then copy and rename your class, then go into your class and change it's "generic arg" to use your new Handler. That's a non-trivial amount of time and cognition for a developer.
Yes, encapsulation is one of the mantras, though IMO this is an acceptable exception simply due to the read-only nature of events. A getter becomes superfluous and is a few extra lines of code that bulks up your editor screen for little gain other than purism (in this case). Thanks so much as well for the links to those GWT dev discussions, very interesting. On Jun 27, 7:05 pm, Thomas Broyer <[email protected]> wrote: > A few comments > > On 27 juin, 13:53, Paul Schwarz <[email protected]> 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/ClassSetupAndInstant...http://code.google.com/p/google-web-toolkit/wiki/AggressiveClinitOpti...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 [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/google-web-toolkit?hl=en.
