Bug 363961

Description Dean Roberts 2011-11-16 14:21:37 EST

Adding a defect to capture information exchanged on the e4-dev mailing list about this topic.

While trying to understand UIEvents and how the topics are built up for subscribe operations I found the buildTopic() 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 the three buildTopic() methods and subscribe using the constants directly.

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.