Community
Participate
Working Groups
API Tools is showing errors for the IBeanObservable, ISWTObservable, and IViewerObservable interfaces and their concrete implementations due to @noextend / @noimplement restrictions. We should introduce observable decorator classes as official API to address this. Various internal classes such as UnmodifiableObservable* should be retrofitted to subclass these classes. The ProxyObservable* classes could probably be eliminated altogether.
+1
Note that this will only remove the errors for the concrete classes. The IBeanObservable, ISWTObservable, and IViewerObservable interfaces will still show the error.
I've been thinking that we could get around the API Tools errors I mentioned in comment #2 by introducing an IObservable subinterface that is meant to be extended / implemented: @noimplement interface IDecoratingObservable extends IObservable { public IObservable getDecorated(); } thus: interface ISWTObservable extends IDecoratingObservable { public Widget getWidget(); } interface IBeanObservable extends IDecoratingObservable { public Object getBean(); public PropertyDescriptor getPropertyDescriptor(); } However there remains the problem of proliferating combinations of interfaces (e.g. ISWTObservable + IObservableValue = ISWTObservableValue), which would still trigger an API Tools error for each subinterface. I think the solution is to proliferate once in the core plugin: @noimplement @noextend interface IDecoratingObservable extends IObservable { public IObservable getDecorated(); } @noimplement interface IDecoratingObservableValue extends IDecoratingObservable, IObservableValue {} @noimplement interface IDecoratingObservableList extends IDecoratingObservable, IObservableList {} @noimplement interface IDecoratingObservableSet extends IDecoratingObservable, IObservableSet {} @noimplement interface IDecoratingObservableMap extends IDecoratingObservable, IObservableMap {} By tagging the subinterfaces as @noimplement but leaving them open for extension, we encourage users to subclass our DecoratingObservable(Value|Set|List|Map) classes rather than implementing the decorating interfaces directly.
Haven't thought this through but - could we just remove the @noextend clause from the observable interfaces, but leave the @noimplement?
(In reply to comment #4) > Haven't thought this through but - could we just remove the @noextend clause > from the observable interfaces, but leave the @noimplement? +1
Created attachment 107154 [details] Prototype Needs tests
Created attachment 107155 [details] mylyn/context/zip
Created attachment 107156 [details] Retrofit existing API to DecoratingObservables
Created attachment 107157 [details] mylyn/context/zip
(In reply to comment #4) > Haven't thought this through but - could we just remove the @noextend clause > from the observable interfaces, but leave the @noimplement? I've removed the @noextend tags but am still getting errors from API tools: Description Resource Path Location Type ISWTObservable illegally implements IObservable ISWTObservable.java org.eclipse.jface.databinding/src/org/eclipse/jface/databinding/swt line 22 API Usage Problem ISWTObservableValue illegally implements IObservableValue ISWTObservableValue.java org.eclipse.jface.databinding/src/org/eclipse/jface/databinding/swt line 21 API Usage Problem IViewerObservable illegally implements IObservable IViewerObservable.java org.eclipse.jface.databinding/src/org/eclipse/jface/databinding/viewers line 22 API Usage Problem IViewerObservableList illegally implements IObservableList IViewerObservableList.java org.eclipse.jface.databinding/src/org/eclipse/jface/databinding/viewers line 21 API Usage Problem IViewerObservableSet illegally implements IObservableSet IViewerObservableSet.java org.eclipse.jface.databinding/src/org/eclipse/jface/databinding/viewers line 21 API Usage Problem IViewerObservableValue illegally implements IObservableValue IViewerObservableValue.java org.eclipse.jface.databinding/src/org/eclipse/jface/databinding/viewers line 21 API Usage Problem It appears that @noimplement is also applied when an interface extends an interface. This surprised me because I thought this is what @noextend would be for. Is this a bug in API tooling?
There is dispute in API tools on whether clients should be allowed to extends a @noimplement interface, even if the implementing class extends a legal implementation of that superinterface.
Created attachment 108307 [details] Refinements to implementation and tests Includes some new, previously unmatched methods in Observables (proxyObservableMap, unmodifiableObservableMap, proxyObservableValue)
Created attachment 108308 [details] mylyn/context/zip
Boris, I think we're close enough to pull the trigger on this one. All tests pass on my machine. I'd like a vote from you before officially introducing the new API though.
Planned for 3.5M2
Created attachment 111732 [details] Updated patch Added a boolean constructor argument to all DecoratingObservable classes that determines whether the decorator, when disposed, will dispose the decorated observable as well. The UnmodifiableObservable* classes pass false to the constructor, the assumption being that these classes are going to be used to make unmodifiable copies that can be passed around to clients, so the client should not be able to dispose the original observable. I've also omitted the BeanObservable*Decorator classes from this updated patch because there are conflicting changes in bug 194734
Created attachment 111733 [details] mylyn/context/zip
Patch released to HEAD > 20080905
Some additional javadoc changes / corrections released to HEAD > 20080905
Verified in I20081209-0100 by code inspection and by running test suite