I'm +1 for this simply because the result would turn:

eventBroker.subscribe(UIEvents.buildTopic(UIEvents.UIElement.TOPIC,
        UIEvents.UIElement.TOBERENDERED), toBeRenderedHandler);

into

eventBroker.subscribe(UIEvents.UIElement.TOBERENDERED, toBeRenderedHandler
);

which, to me at least, is cleaner and directly accessible using code 
completion... As Brian points out making initial users 'wince' isn't 
really the goal...;-). Perhaps now is the time to address it, by the time 
4.2 goes out it *will* be too late.

The down side is that this would be a *BREAKING CHANGE*, if we choose to 
go this route then we should eat the pain now. If we decide it's necessary 
we might 'deprecate' buildTopic and change it to simply return the value 
of the 'attrName' but it should be removed in M5 (I do not want to release 
initial API that already has deprecated stuff in it).

If anybody else is using these APIs (we know Brian is) speak up. Note that 
the change needed to catch up is trivial but we can give a crossover 
'grace' period if you need it.

Onwards,
Eric




From:
Brian de Alwis <[email protected]>
To:
E4 Project developer mailing list <[email protected]>
Date:
09/23/2011 07:12 AM
Subject:
Re: [e4-dev] Suggestion to restructure UIEvents to increase clarity and 
performance
Sent by:
[email protected]



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


_______________________________________________
e4-dev mailing list
[email protected]
https://dev.eclipse.org/mailman/listinfo/e4-dev

Reply via email to