Community
Participate
Working Groups
Before data binding goes 1.0 it would be wise to provide EventObjects for the following listeners to ensure that the API doesn't preclude future expansion of the values supplied to listeners. Interfaces that would be affected: * IValueChangeListener * IValueChangingListener * IListChangeListener * ISetChangeListener * IMappingChangeListener
The diff objects are extensible, but the events are not. I thought about this, but couldn't think of any examples where you wanted to provide more information in an event. Do you have any examples in mind?
(In reply to comment #1) > Do you have any examples in mind? Not off the top of my head. The only reason I bring it up is that I've been burned by this in the past. If the listeners are left as is and down the road we want to add another parameter we have to make a nonpassive change or create another interface. If we had EventObjects for each we'd at least be leaving the door open to the possibility of adding a value in the future.
Yes I know, but the way this is designed, I think that if there are new kinds of events, we will be able to add them as new addXXXListener/removeXXXListener pairs. (I just released additional Javadoc for the IObservable interfaces saying that they must not be implemented by clients directly.) If there is something about the change that needs to be added to the events, we can put that on the diffs. Our design choice was that we wanted fully typed listeners (as opposed to untyped listeners like SWT's listener). Closing as WONTFIX - if you come across a good example of something we might want to add, feel free to reopen this.
Thanks for the explanation; it's much appreciated.
I have been thinking about cracking this one open again for some time now... It seems that we need event objects, not for extensibility, but for being able to deliver events to listeners in other orders than depth-first (the current behaviour). I don't think we should solve the event ordering problem now, but I would like to be in a position where we can solve this later. My thinking is to do the following: - Change every handle<kind>Change(IObservable<kind> source, I<kind>Diff diff) to handle<kind>Change(<kind>ChangeEvent event) - Create <kind>ChangeEvent classes with public fields for IObservable<kind> source and I<kind>Diff diff - Create types and methods for managing untyped listeners and untyped events, but without exposing this to clients (just to implementers of IObservable) I am still pondering what the best API is for the third part of this is. One idea would be to turn the listener interfaces into abstract classes, and let these abstract classes extend a class AbstractChangeListener: public abstract class AbstractChangeListener { protected abstract void handleChange(AbstractChangeEvent event); } ValueChangeListener would then look like this: public abstract class ValueChangeListener extends AbstractChangeListener { final protected void handleChange(AbstractChangeEvent event) { Assert.isTrue(event.kind == ValueChangeEvent.KIND); handleValueChange((ValueChangeEvent) event); } protected abstract void handleValueChange(ValueChangeEvent event); } The other side could look like: public class ChangeEventSupport { protected ChangeEventSupport(Realm realm) { Assert.isNotNull(realm); this.realm = realm; } protected void addChangeListener(String kind,AbstractChangeListener listener){ ... } protected void removeChangeListener... protected boolean hasListeners() { ... } protected void fireChangeEvent(AbstractChangeEvent event) { ... // this event can be enqueued, for example in the realm } } public class AbstractObservableValue extends ChangeEventSupport { public void addValueChangeListener(ValueChangeListener listener) { addChangeListener(ValueChangeListener.KIND, listener); } public void removeValueChangeListener... protected void fireValueChange(ValueDiff diff) { fireChangeEvent(new ValueChangeEvent(this, diff)); } } The class ChangeEventSupport could be used as a base class for observables, or in cases where we want to extend some other class (like e.g. AbstractList), a ChangeEventSupport object could be created in the constructur so that the add/removeListener and fire...Event methods can be delegated to that ChangeEventSupport object. Did you manage to read everything up until here? What do you think?
> Did you manage to read everything up until here? What do you think? I managed to read everything up to there three times. I understood what you were getting at the 3rd time. :-) Two thoughts right now: 1) I like the idea a lot. 2) Make sure it's possible for clients to create event objects from scratch to fire at observables inside unit tests. In SWT, it's not possible to do this from outside the SWT hierarchy because of e.widget being of type Widget. (See bug #138997)
I think I followed. I like the idea of event objects but I'm curious what the need is for untyped events and listeners. From your comments I gathered that it's for the maintenance but want to be sure. Could we expose an interface to the client with an optional abstract implementation? I prefer an interface to allow the consumer to implement it as a mixin interface rather than force them to have to have separate objects per listener. We'd then be able to determine if the listener is an instance of the abstract class and if not wrap it with an object that would delegate to the listener.
(In reply to comment #7) > We'd then be able to determine if the listener is an instance of the > abstract class and if not wrap it with an object that would delegate to the > listener. Just so I remember when I come back to this: What Brad describes above happens at the time a listener is added. When a listener is removed, we will have to find the wrapper and remove the wrapper. For this to work, clients need to call removeListener() with the exact same listener that they passed when calling addListener(), they cannot just pass one that is equals(). I'm fine with this, but it is worth mentioning here because it needs to go into the Javadoc for add/removeListener.
Fix released >20061211. I didn't need any listener wrapper objects because the event object can be used to dispatch the event to the listeners in a type-safe way.
Verified that my changes are in I20061213-0800.
