Community
Participate
Working Groups
Copied from #eclipse-dev discussion between Tom Schindl and Matthew Hall: (10:10:02 AM) tomsontom: qualifafial: by the way EMFLObservablesMap works a lot better than the one from BeanObservables (10:10:35 AM) Matthew Hall: tomsontom: what is BeansObservables doing incorrectly? (10:10:39 AM) tomsontom: e.g. in case of the new tree support it can deal with different object types and not like the Bean one with only one type (10:11:03 AM) tomsontom: it doesn't do anything incorrect (10:11:54 AM) Matthew Hall: tomsontom: Can EMF share structural features between types without them having a common interface? (10:12:09 AM) tomsontom: but the EMF one can work with mixed object types (say the IObservableList has A.class and B.class) (10:12:40 AM) tomsontom: that's the key in there it doesn't depend on a the interface because it works on the generic one (10:12:48 AM) tomsontom: EObject (10:13:32 AM) tomsontom: one of the biggest limitation of the current Tree implementation is that the content implementation from you can deal with different object types (10:14:03 AM) Matthew Hall: So an EStructuralFeature can be used interchangeably between different types as long as both types have a feature with the same properties? (10:14:11 AM) tomsontom: whereas the BO#observeablesMap requires the classes to derive from one base (10:14:31 AM) tomsontom: no (10:15:07 AM) tomsontom: the problem with the BO one is that the first thing it does is to create a JavaBeanObservable which requires the class name (10:15:14 AM) tomsontom: or better the class (10:15:55 AM) Matthew Hall: I've been hacking at those--the reason it needs that is to create a java.beans.PropertyDescriptor, which needs the bean class (10:16:15 AM) Matthew Hall: or rather, it needs the bean class so it can use Introspector to get the PropertyDescriptor (10:17:30 AM) tomsontom: couldn't we delay the creation (10:18:34 AM) Matthew Hall: possibly--but currently they delegate to the property descriptors so that's why the restriction is there (10:18:43 AM) tomsontom: or better pass a special observable something like ConditionalObservable (10:20:20 AM) Matthew Hall: btw, I've all but eliminated the beans observable implementations as part of the properties patch. Now they are just PropertyObservables wrapped in a DecoratingObservable* that implements IBeanObservable (10:20:45 AM) tomsontom: hm I think I really need to take a look into this (10:21:17 AM) tomsontom: I'd like to eliminate the BO#observeableMap limitation in 3.5 if I have time (10:33:42 AM) Matthew Hall: Maybe in BeanProperties we could implement an anonymous property (property name but no bean class) that allocates and delegates to a concrete property when the bean class is known. (10:34:37 AM) tomsontom: sounds interesting could you file a bug and then we can share our ideas there
JavaBeanObservableMap, in the new properties bug, is actually a IObservableMap which observes an IValueProperty over the elements of an IObservableSet. My first idea is to implement a value property which only has a property name, and no bean class. On a case by case basis, as the property is used to accessed/modified/listen to various beans, we should allocate concrete bean properties and delegate to them. One question I have: what should we do when a bean is added to the domain set doesn't have the named property? Should get(bean) return null, and set(bean, value) do nothing, or should an error be thrown? As far as user-friendliness and programmer-friendliness I would assume that an unknown property should just be ignored.
That's what the EMFObservableMap currently does, if you try to get a value from an EObject which doesn't contain the EStructuralFeature you simply get a null back.
Created attachment 113033 [details] Something like this
Created attachment 113034 [details] mylyn/context/zip
Created attachment 113038 [details] Cache the property delegate even if null to avoid repeating the delegate creation process every time we access a bean lacking the given property
Created attachment 113039 [details] mylyn/context/zip
Also added this code to BeanProperties: /** * Returns a value property for the given property name of an arbitrary bean * * @param propertyName * the property name * @return a value property for the given property name of an arbitrary bean */ public static IValueProperty valueProperty(String propertyName) { return valueProperty(propertyName, null); } /** * Returns a value property for the given property name of an arbitrary bean * * @param propertyName * the property name * @param valueType * the value type of the returned value property * @return a value property for the given property name of an arbitrary bean */ public static IValueProperty valueProperty(String propertyName, Class valueType) { return new GenericBeanValueProperty(propertyName, valueType, true); } PojoProperties would get corresponding additional methods as well
EMF has also small problems with such a feature which are tracked in bug 248069
Moving to M5
Created attachment 120773 [details] Patch against mhall_bug294734_properties branch This turned out to be pretty straightforward to write. To give it a spin, checkout the mhall_bug294734_properties branch, then apply this patch, then take Snippet025 for a spin. Note that generic properties have an inherent overhead for lookups whenever you observe a value property over an observable list, set or map. This is because you have to look up the delegate to interact with each element in the observable. Whereas with value properties the lookup only has to be done once, at the time that observables are created, so performance should not be affected in this case. Based on what it took to write this patch, it shouldn't be too hard to implement generic list, set and map properties as well.
Created attachment 120774 [details] mylyn/context/zip
Created attachment 120790 [details] Update Generic bean/pojo properties are now supported for list, set and map properties in this patch Still need to put generic methods into BeansObservables and PojoObservables
Created attachment 120791 [details] mylyn/context/zip
Created attachment 120907 [details] Update Added methods to BeansObservables and PojoObservables: public static IObservableMap observeMap(IObservableSet domain, String propertyName); public static IObservableMap observeMaps(IObservableSet domain, String propertyName[]); Tom, does this satisfy your original requirement?
Created attachment 120908 [details] mylyn/context/zip
Created attachment 121022 [details] Update Refactored Delegating<Value|List|Set|Map>Property out of Generic*Property. Renamed Generic*Property to Anonymous*Property Added Snippet026AnonymousBeanProperties.java snippet. This snippet demonstrates a tree viewer with elements of different types and no common ancestor class (ContactGroup, Contact), and different subsets of properties (i.e. both have a "name" property but only Contact has a "status" property).
Created attachment 121023 [details] mylyn/context/zip
Created attachment 121165 [details] Merge latest patch with tip of mhall_bug194734_properties branch
Created attachment 121166 [details] mylyn/context/zip
Created attachment 121171 [details] Update Delegate Property.getDefaultRealm(Object) to correct delegate in Delegating*Property classes
Created attachment 121172 [details] mylyn/context/zip
Created attachment 121277 [details] Updated contribution comments, small revisions to snippet
Created attachment 121278 [details] mylyn/context/zip
Created attachment 121589 [details] Work in progress Because of work committed from 195222 I am changing course on this bug. The Delegating*Property class will be supporting all properties, not just extensions of Simple*Property. Note that this patch has compiler errors
Created attachment 121590 [details] mylyn/context/zip
Created attachment 121648 [details] Work in progress Need to finish implementing ObservableMapDelegatingValuePropertyObservableMap
Created attachment 121649 [details] mylyn/context/zip
Created attachment 121728 [details] First draft The Observable<Map|Set|List>DelegatingValuePropertyObservable<Map|List> classes need close scrutiny and some testing before signing off on them.
Created attachment 121729 [details] mylyn/context/zip
Created attachment 121763 [details] Further refactoring Unified the caching mechanism between Observable<List|Set|Map>DelegatingValuePropertyObservable<List|Map> classes
Created attachment 121764 [details] mylyn/context/zip
Created attachment 121770 [details] Revised snippet
Created attachment 121771 [details] mylyn/context/zip
Created attachment 121867 [details] Backup Just backing up the current state before checking out another branch...
Created attachment 121868 [details] mylyn/context/zip
Released latest patch to mhall_bug194734_properties branch
Work on this feature is complete for all intents and purposes. All that remains is for the mhall_bug194734_properties branch to be merged into HEAD. Anyone wanting to follow this feature should CC themselves to bug 194734 in order to follow the progress of that bug.
Released to HEAD (as part of bug 194734) > 20090120
Congrats to get this work in HEAD! Thanks for the hard work Matt.
Thanks Tom! It's been a fun project, but after six months I'm glad this is finally DONE. :)
Created attachment 123158 [details] Add BeanProperties.values(String[]), PojoProperties.values(String[])
Created attachment 123159 [details] Add BeanProperties.values(String[]), PojoProperties.values(String[])
Created attachment 123160 [details] Add BeanProperties.values(String[]) and PojoProperties.values(String[])
Created attachment 123161 [details] mylyn/context/zip
Verified in I20090129-0100 by running Snippet026AnonymousBeanProperties
