I remember being baffled when trying to figure out events. I managed to cobble
something together for the Cocoa UI processor from snarfing bits of code from
elsewhere, but I wince in pain whenever I revisit the code.
So FWIW, I support Dean's proposal.
Brian.
On 22-Sep-2011, at 3:30 PM, Dean Roberts wrote:
> While trying to understand UIEvents and how the topics are built up for
> subscribe operations I found the buildTopic() methods a little cumbersome and
> a barrier to understanding what was really going on.
>
> I would like to make a suggestion that I think would
>
> 1) Make the code look cleaner
> 2) Make the concepts clearer
> 3) Improve run time performance (albeit marginally)
>
> For illustration purposes, here is an example of an interface definition from
> UIEvents for the UIElement model object generated by GenTopic
>
> public static interface UIElement {
> public static final String TOPIC = UIModelTopicBase +
> "/ui/UIElement"; //$NON-NLS-1$
> public static final String ACCESSIBILITYPHRASE =
> "accessibilityPhrase"; //$NON-NLS-1$
> public static final String CONTAINERDATA = "containerData";
> //$NON-NLS-1$
> public static final String CURSHAREDREF = "curSharedRef";
> //$NON-NLS-1$
> public static final String ONTOP = "onTop"; //$NON-NLS-1$
> public static final String PARENT = "parent"; //$NON-NLS-1$
> public static final String RENDERER = "renderer";
> //$NON-NLS-1$
> public static final String TOBERENDERED = "toBeRendered";
> //$NON-NLS-1$
> public static final String VISIBLE = "visible"; //$NON-NLS-1$
> public static final String VISIBLEWHEN = "visibleWhen";
> //$NON-NLS-1$
> public static final String WIDGET = "widget"; //$NON-NLS-1$
> }
>
> To subscribe to an event that tracks a UIElements visible change you would
> write
>
> eventBroker.subscribe(UIEvents.buildTopic(UIEvents.UIElement.TOPIC,
> UIEvents.UIElement.VISIBLE), visibilityHandler);
>
> To subscribe to all changes to UIElement you would write
>
> eventBroker.subscribe(UIEvents.buildTopic(UIEvents.UIElement.TOPIC),
> visibilityHandler);
>
> I think it would be much nicer to write, for the first case,
>
> eventBroker.subscribe(UIEvents.UIElement.VISIBLE), visibilityHandler);
>
> or, for the second case,
>
> eventBroker.subscribe(UIEvents.UIElement.ALL), visibilityHandler);
>
> To make this possible I propose that we would generate the UIElement
> interface definition like this.
>
> public static interface UIElement {
> public static final String ALL= UIModelTopicBase +
> "/ui/UIElement/*"; //$NON-NLS-1$
> public static final String VISIBLE = UIModelTopicBase +
> "/ui/UIElement/" + "visible"; //$NON-NLS-1$
> ...
> }
>
> This would allow us to delete two of the buildTopic() methods. We could
> leave the 3 argument version of buildTopic() to retain construction of topics
> that filter based on event type ( although nobody calls this internally at
> this time so do we really need it?).
>
> The code would also be more efficient as we would be removing method calls
> that do string concatenation with simple static references. In fact, I
> believe the compiler will just in line the references at compile time.
>
> Although I would be a fan of simply making these changes and requiring
> consumers to make a few small simplifying changes, we could provide a
> stepping stone by regenerating the new shape, deprecating the buildTopic()
> methods and all the TOPIC fields. We would then change the implementation of
> the buildTopic methods to return the appropriate constant directly without
> concatenating.
>
> Admittedly there is higher priority work to be done but since this class is
> essentially API, if we don't change it now, we will never be able to make the
> change. And I think the clarity in the subscribe statements make the change
> worth it. This change seems to fit well with the Eclipse 4 mantra that
> simple things should be simple.
>
> I also believe the effort required to change GenTopic to generate the new
> shape is small, and I volunteer to create the patch. Admittedly changing the
> calling code to use the new shape is more wide spread, but a very simple
> task. Although an actual committer probably wants to do that work as it
> would be a LOT of patches to apply :-)
>
> What does the community think? Does it make sense? Am I totally off base?
>
> [email protected]
> _______________________________________________
> e4-dev mailing list
> [email protected]
> https://dev.eclipse.org/mailman/listinfo/e4-dev
_______________________________________________
e4-dev mailing list
[email protected]
https://dev.eclipse.org/mailman/listinfo/e4-dev
