Community
Participate
Working Groups
In our initial
Sorry--hit enter too soon in the wrong place. In our initial design we had a static DataBinding factory for DataBindingContext objects. We abandoned this design because we felt it constrained us from evolving the API. The set of Observables created by the standard factories could never be changed after a given release. The replacement design was to push responsibility for creating and configuring a data binding context onto each application developer. However, this is problematic in several other ways: 1) It will be a support concern. Whenever someone comes to us with data binding code that they claim doesn't work, we'll have to ask them to send us the code they used to initialize their data binding context. 2) It is a growing maintanence burdern. As we evolve our API, the set of standard factories grows. Each snippet we write for our documentation repeats this same boilerplate chunk of data binding context initiailization code. Each snippet will need to be updated every time we add to or change the way these default factories work. The more snippets we have, the more of a burden it will be to keep each snippet updated with the current state of data binding. I suggest the following changes to improve this situation: 1) Have a strategy pattern interface IDataBindingContextFactory for things that can create data binding contexts. 2) Make the static DataBinding class keep a map of these factories keyed by plugin ID so that separate plugins can use their own IDataBindingContextFactories. When you use a factory, you now must specify the ID of the plugin whose factory you want to use. You must explicitly register a factory with the ID before you can use it. 3) Define API update policy for what changes are allowed. Basically I think that this means that once we release a factory as API, the set of things it creates can never change or those changes could potentially break downstream clients. So we'll have SWTObservableFactory1, SWTObservableFactory2, and so on. This is a bit ugly, but not horrible. But it would enable us to: 4) Define a standard IDataBindingContextFactory implementation. The implementation could be designed for subclassing so that folks that just want the standard stuff plus a few of their own custom factories can express this easily. They also could easily register this factory with the DataBinding factory registry. Or use it themselves. For each successive non-breaking API release, we can subclass this factory ourselves to add whatever changes we need to make without breaking downstream clients that rely on the current behavior. I think this pretty much would do what we need. Are there any thoughts/suggestions before I code it up?
Stefan wrote via email: ------ I'm using observables as the common currency for all my models and viewers to talk with one another. I've been avoiding DataBindingContext because of the overhead in creating it (the problem described below). I believe we should break apart DBC's responsibilities, such that nobody needs to create or initialize a global DBC. The current DBC seems to be responsible for many different things: - Acts as a factory (or a collection of factories) for observables. This responsibility can be removed from DBC and moved to static factory methods. The pattern I described for SWT observables would also apply to other observable types. Compare the following snippets. // DBC as a factory for observables // Advanatages: // - Slightly less code to create the binding // Disadvantages: // - A lot of code to initialize a global context and maintain a hierarchy of contexts // - Refactoring the global dbc can easily break clients of it - for example, it would be // very hard to tell if anyone depends on a deprecated factory registered with the global // context. dbc.bind(myTextbox, new Property(myBean, "propertyId")); // DBC as a container for bindings. It binds to observables it is given, but does not create observables. // Advantages: // - No global DBC to be initialized. // - No magic. Easy to track the dependencies on the DBC, and the methods that create each observable // can provide clear contracts about the type of observable created and its value type. // Disadvantages: // - Slightly more code to create each binding. dbc.bind(SwtObservables.getText(myTextbox), BeanObservables.getBean(myBean, "propertyId")); - Manages the lifecycle for a set of observables. This may be a useful facility, but it is independent of DBC. In every case I've wanted to use them, the lifecycle of the observables was not the same as the lifecycle of the bindings. If we wanted a helper class for managing the lifecycle of observables (much like ResourceManager does for SWT resources), we could offer this as a separate helper class. - Manages the lifecycle of a set of bindings, and collects their validation messages. This is good stuff. This is actually the only thing I expected DBC to do. This does not require a global DBC or even a hierarchy of them - clients can create one for each editor, wizard page, or dialog they create. Unfortunately, clients need to initialize their DBC to act as a factory for various things even if they are only using it to manage a set of bindings. - Acts as a factory for bindings. It is reasonable to include a helper method that creates 1-to-1 bindings that convert between java primitives and provides simple type validation... but there is no need to have it act as a an extensible registry of binding factories. If we wanted a general-purpose binding factory registry, we could still have one... but it doesn't need to be embedded in DBC. Basically, if DBC were responsible for managing a set of bindings and their validation states, we could move all the other functionality elsewhere. In this case, clients wouldn't need to create any global context in order to use the framework. - Stefan Brad Reynolds <bradleyjames@gmail.com> 06/14/2006 11:39 PM To Stefan Xenos/Ottawa/IBM@IBMCA cc "David J. Orme" <djo@coconut-palm-software.com>, Boris Bokowski/Ottawa/IBM@IBMCA, sxenos@gmail.com Subject Re: Snippets started I'm curious about the fact that you left the DBC out of this. Are you not using bindings? What is on the model side of the equation? I've been trying to attack this from the DBC's perspective and haven't been too happy with it. I created methods on the DBC to retrieve: * observables for a description (multiples are returned if they exist) * the binding for an observable * observables for a binding I was then storing a reference to the DBC on the target if it was a Widget. I don't like this for obvious reasons. Regardless of what I end up doing is this of any concern to you? Would having access to this information at the DBC level help you out? Thanks, Brad On Jun 12, 2006, at 3:01 PM, Stefan Xenos wrote: IMHO, we should include helpers like the following for all of our SWT and JFace observables. This makes them really easy to use without the need for all that boilerplate. /** * Returns an IObservableValue of type Boolean * * @param theControl * @return */ public static IObservableValue getSelection(Button theControl) { return (IObservableValue) getObservable(theControl, SWTProperties.SELECTION); } /** * Returns an IObservableValue of type Boolean * * @param theControl * @return */ public static IObservableValue getEnablement(Control theControl) { return (IObservableValue) getObservable(theControl, SWTProperties.ENABLED); } /** * Returns an IObservableValue of type Boolean * * @param theControl * @return */ public static IObservableValue getVisible(Control theControl) { return (IObservableValue) getObservable(theControl, SWTProperties.VISIBLE); } /** * Returns an IObservableValue of type String * * @param theControl * @return */ public static IObservableValue getText(Text theControl) { return (IObservableValue) getObservable(theControl, SWTProperties.TEXT); } I use the following implementation for getObservable: private static final class BindingRec { private Object object; private IObservableFactory factory; private Map observables = new HashMap(); public BindingRec(Object target, IObservableFactory theFactory) { object = target; factory = theFactory; } public IObservableFactory getFactory() { return factory; } public IObservable getObservable(Object attribute) { IObservable result = (IObservable) observables.get(attribute); if (result == null) { result = factory.createObservable(new Property(object, attribute)); observables.put(attribute, result); } return result; } public void dispose() { IObservable[] toDispose = (IObservable[]) observables.values().toArray(new IObservable[observables.values().size()]); for (int i = 0; i < toDispose.length; i++) { IObservable value = toDispose[i]; value.dispose(); } } }; private DatabindingUtil() { } /** * Returns the data binding context for the given control. * * @param control the control * @return */ /* package */ static BindingRec getBindingRec(Control control) { BindingRec rec = (BindingRec)control.getData(CONTEXT_ID); if (rec == null) { final BindingRec newRec = new BindingRec(control, new SWTObservableFactory()); rec = newRec; control.addDisposeListener(new DisposeListener() { public void widgetDisposed(DisposeEvent e) { newRec.dispose(); } }); } return rec; } /** * Returns an observable property of the given control. Supports any property * that can be created by SWTObservableFactory. See SWTProperties for a list * of property IDs. Callers may downcast the IObservable to the correct type * that corresponds to their property ID. Callers must not dispose the returned * observable. * * @param theControl * @param property a property ID from org.eclipse.jface.internal.databinding.provisional.swt.SWTProperties * @return an IObservable */ /* protected */ static IObservable getObservable(Control theControl, Object property) { BindingRec rec = getBindingRec(theControl); return rec.getObservable(property); }
If the core data binding framework is separated from the Javabeans and the SWT/JFace stuff, does your plan still work?
My thoughts about Stefan's proposal: I'm seeing more and more cases where the order in which observables are updated matters and where you can't use Display#asyncExec() to hack the order you want without introducing infinite update loops (because our updating flags get set back to false before the Runnable runs.) I like the clarity of Stefan's proposal but is there a way to do this while maintaining the ability to have a cenrtalized updater that can manage situations like this for us?
(In reply to comment #3) > If the core data binding framework is separated from the Javabeans and the > SWT/JFace stuff, does your plan still work? If the master factory is in its own "master" data binding plugin that requires all the baseline plugins, it would still work. However I find Stefan's proposal thought-provoking and don't want to start coding until we've hashed this out some more.
More thoughts: 1) The part I don't like about Stefan's proposal is that it requires the client to be familiar with the specific factory that creates each and every IObservable. This is akin to having to remember the specific package in which each and every Java class lives. Just like "Organize imports" is your friend inside Eclipse, I think having a generic factory handling observable construction is also a good idea. 2) In view of the above, I've implemented a first cut at a binding factory as discussed in comment #1, and upgraded the current set of snippets to use it. Here's the new Hello World snippet using the new data binding factory: public class Snippet000HelloWorld { public static void main(String[] args) { ViewModel viewModel = new ViewModel(); Shell shell = new View(viewModel).createShell(); // The SWT event loop Display display = Display.getCurrent(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) { display.sleep(); } } // Print the results System.out.println("person.getName() = " + viewModel.getPerson().getName()); } // The data model class. This is normally a persistent class of some sort. // // In this example, we only push changes from the GUI to the model, so we // don't worry about implementing JavaBeans bound properties. If we need // our GUI to automatically reflect changes in the Person object, the // Person object would need to implement the JavaBeans property change // listener methods. static class Person { // A property... String name = "HelloWorld"; public String getName() { return name; } public void setName(String name) { this.name = name; } } // The View's model--the root of our Model graph for this particular GUI. // // Typically each View class has a corresponding ViewModel class. // The ViewModel is responsible for getting the objects to edit from the // DAO. Since this snippet doesn't have any persistent objects to // retrieve, this ViewModel just instantiates a model object to edit. static class ViewModel { // The model to bind private Person person = new Person(); public Person getPerson() { return person; } } // The GUI view static class View { private ViewModel viewModel; public View(ViewModel viewModel) { this.viewModel = viewModel; } public Shell createShell() { // Build a UI Shell shell = new Shell(Display.getCurrent()); shell.setLayout(new RowLayout(SWT.VERTICAL)); Text name = new Text(shell, SWT.BORDER); // Bind it DataBindingContext bindingContext = new DataBindingFactory1().createContext(shell); Person person = viewModel.getPerson(); bindingContext.bind(name, new Property(person, "name"), null); // Open and return the Shell shell.pack(); shell.open(); return shell; } } }
(In reply to comment #6) > 1) The part I don't like about Stefan's proposal is that it requires the client > to be familiar with the specific factory that creates each and every > IObservable. This is akin to having to remember the specific package in which > each and every Java class lives. Just like "Organize imports" is your friend > inside Eclipse, I think having a generic factory handling observable > construction is also a good idea. This is actually the part that I like. One of the difficulties that I had when evaluating databinding was knowing what to pass to the DBC.bind(...) method. By having the one generic method it's difficult to document what is required for a binding to occur so I had to step through the code and get familiar with the factories which is what the generic bind method was created to avoid. Stefan's proposal provides a mechanism to quickly know what the parameters are and to also document those parameters. I don't think that everything needs API like Stefan's proposing but I do think that for the common SWT, JFace, and Beans it's beneficial to have these as it is expressive, creates the possibility for documentation where the developer needs it (in javadoc), and simplifies the debug/maintenance process.
Sorry - I've been away for a bit and am just now catching up on my bugs and emails. > If the core data binding framework is separated from the Javabeans and > the SWT/JFace stuff, does your plan still work? It actually makes this easier. If you don't have any global DBC, then nobody needs to know about the complete set of observable factories. You can move the observables for SWT/JFace into a separate plugin and nobody needs to know about them but the client code that binds to them. > I like the clarity of Stefan's proposal but is there a way to do > this while maintaining the ability to have a cenrtalized updater that > can manage situations like this for us? I agree that there is a need for a centralized updater in order to manage the order of events. Boris and I have been discussing several algorithms for this (and I suspect you folks may have your own ideas as well)... but a centralized event manager could just be a singeton provided by the framework. It does not need to be part of data binding context. Although important, I believe that this is an independent issue and that we can discuss it in a separate PR. > The part I don't like about Stefan's proposal is that it requires the > client to be familiar with the specific factory that creates each and > every IObservable. I suspect that many programmers will consider this a benefit. Most apps will not use a lot of factories, and the ones they use will be used frequently. For example, my app uses EMF for its model and SWT for the UI, so I'd mainly be using EMFObservables and SWTObservables. Memorizing two frequently used class names is not a problem for most programmers. The alternative is to memorize property IDs, I personally find it a lot easier to memorize a class name than a property ID.
One of the things we do at TPC is use metafactories. So our current DBC factory looks something like this: public DataBindingContext createContext(int textBindingTime) { DataBindingContext context = new DataBindingContext(); BeanObservableFactory beanObservableFactory = new BeanObservableFactory(context, null, null); context.addObservableFactory(new NestedObservableFactory(context)); context.addObservableFactory(beanObservableFactory); context.addObservableFactory(new StringTrimObservableFactory(beanObservableFactory)); context.addObservableFactory(new PartnerControlsObservableFactory()); </snip/> return context; } Notice the StringTrimObservableFactory. This is a meta factory for a meta observable that wraps bean observable values with a String data type. It is higher in the search priority, so any binding that creates a bean observable value will automatically get string trim observable behavior. The behavior is simple: if the string after trimming is an empty string "", the value is replaced with null. This provides a generic translation layer between SWT semantics (getText() never returns null) and database semantics (where you want empty strings to be null. It's like having AOP for observable values. I've also implemented generic Eclipse undo/redo support (in a previous version of data binding) this way. Unless I'm mistaken, requiring developers to create their observables directly will eliminate the ability to wrap observables and bindings as a way to generically add behavior to an application.
(In reply to comment #9) > Unless I'm mistaken, requiring developers to create their observables directly > will eliminate the ability to wrap observables and bindings as a way to > generically add behavior to an application. Yes - I believe by not using factories, this ability will be eliminated.
> The behavior is simple: if the string after trimming is an empty string "", > the value is replaced with null. This provides a generic translation layer > between SWT semantics (getText() never returns null) and database semantics > (where you want empty strings to be null. ...doing this in DBC isn't the only way. The programmer who created the binding could have done this when they created the binding: dbc.bind(new StringTrimObservable(SwtObservables.getText(myTextbox)), DatabaseObservables.create(...)); The difference is that with the above code, there is no possibility of a change in the global DBC affecting what binding is created - whereas with the global DBC, changes to the DBC can affect bindings througout the app. To you, the ability to do these AOPish substitutions is a feature. You expect your team to have a deep enough understanding of the binding rules that they can profit from them and debug them. Your app will be using declarative patterns to describe how SWT controls talk to databases, etc. To me, the fact that these substitutions are impossible is a feature. My team doesn't have this depth of knowledge of the binding rules and my manager won't approve the training required to change this. I would not want to debug bindings that depended on subtleties of the lookup rules in the global DBC - or worry about the impact of changing the global DBC when any change could affect any binding in the system. To me, this restriction is a feature in the same sense that having a private section in my classes is a feature - it prevents you from doing certain things, but this also simplifies maintenance. My app will use traditional OOP to create their bindings. This doesn't need to be an all-or-nothing proposition. If we break up DBC, it should be possible refactor the code such that we can both use our preferred patterns without either of us forced into the other approach. 1. Break up the responsibilities of DBC as I suggested in comment 2 - DBC only holds a set of bindings and their validation states. 2. Offer the convenience methods for creating observables directly. 3. Create a new MetaFactory utility class. If the application developer wishes to use a singleton metafactory for obtaining all their app's observables, they may do so, and they'll get all the AOP-ish behaviors you desire. If the application developer wishes to obtain their observables directly, they may do so - and then they can rely on not having to debug those same AOP-ish behaviors. In this case you can do it your way: public MetaFactory createContext(int textBindingTime) { MetaFactory context = new DataBindingContext(); BeanObservableFactory beanObservableFactory = new BeanObservableFactory(context, null, null); context.addObservableFactory(new NestedObservableFactory(context)); context.addObservableFactory(beanObservableFactory); context.addObservableFactory(new StringTrimObservableFactory(beanObservableFactory)); context.addObservableFactory(new PartnerControlsObservableFactory()); </snip/> return context; } // ...and creating bindings could look like this: MetaFactory mf = MyMetaFactory.getInstance(); dbc.bind(mf.getObservable(myTextbox), mf.getObservable(new Property(myBean, "propertyId"))); // ...or perhaps this (if you used ...say a MetaBindingsFactory for bindings instead // of a metafactory for observables) MetaFactory mf = MyMetaFactory.getInstance(); dbc.addBinding(mf.bind(myTextbox, new Property(myBean, "propertyId"))); ...and I can do it my way: // Bindings look like this - there is no global initialization step and // no possibility of globally replacing these observables with different // implementations. dbc.bind(SwtObservables.getText(myTextbox), BeanObservables.getBean(myBean, "propertyId")); I *think* this should be a reasonable comprimise.
> Unless I'm mistaken, requiring developers to create their observables directly > will eliminate the ability to wrap observables and bindings as a way to > generically add behavior to an application. > Yes - I believe by not using factories, this ability will be eliminated. Quite true - but for me this is a feature. Just as having my fields declared as private prevents them from being modified elsewhere in the application, preventing my observables from being changed elsewhere in the app is a good thing. My team is not trained in the use of AOP, and can use other techniques to reuse general behaviors.
(In reply to comment #11) > dbc.bind(SwtObservables.getText(myTextbox), > BeanObservables.getBean(myBean, "propertyId")); I am beginning to think that this would be a good thing. Dave, Brad, Peter, what about you? - it makes the API smaller and easier to understand - the Javadoc will be easier to write since there is no magic - binding is still just one line of code - no need for a "standard factory" that would make it hard to separate the framework into several plug-ins (core framework, SWT/JFace support, Beans support)
(In reply to comment #13) > I am beginning to think that this would be a good thing. Dave, Brad, Peter, > what about you? +1
(In reply to comment #13) > (In reply to comment #11) > > dbc.bind(SwtObservables.getText(myTextbox), > > BeanObservables.getBean(myBean, "propertyId")); > > I am beginning to think that this would be a good thing. Dave, Brad, Peter, > what about you? > > - it makes the API smaller and easier to understand I think there are a few things we might have overlooked here. In order to support undo/redo, you have to always remember to write something like the following *everywhere*: dbc.bind(SwtObservables.getText(myTextbox), UndoRedoObservables.getUndoRedo( BeanObservables.getBean(myBean, "propertyId"))); Except it's not even that simple. For our app, this becomes: dbc.bind(SwtObservables.getText(myTextbox), UndoRedoObservables.getUndoRedo( StringTrimObservables.getStringTrim( (BeanObservables.getBean(myBean, "propertyId")))); 1) This gets pretty cumbersome to write pretty fast. 2) And you have to repeat this same code *everywhere*. 3) 50% of that code is boilerplate, which violates the DRY principle (don't repeat yourself). 4) Mixing all of that configuration code into every single binding makes it much harder to see the application logic peeking out from the middle of all of that configuration logic. :-) It gets worse when you try to bind to a TableViewer or CompositeTable. When you bind to Tabular things like a TableViewer with CellEditors or a CompositeTable you're not explicitly binding the table cell editors. Whenever a cell editor is activated, it is bound separately by the table observable. There is a similar problem with CompositeTable. CompositeTable's virtual list observable needs to have a factory it can call to generically bind any arbitrary SWT control that it finds inside a row object. It doesn't know if that binding is going to come from a SWTObservables factory, a BeanObservables factory, a ViewerObservables factory, or some custom factory associated with some custom control that the project has created. So it needs a generic way to search all the applicable factories to find one that can create an appropriate observable. And it needs a generic description object telling it what to bind that control to. And it needs a generic API described by some interface that it can call to do all this. This does away with using static methods as factories like is being proposed since the column binding factories will need to be passed into the binding. It also means that we will not be able to eliminate the description objects from the API It also means that in addition to passing column description objects to tabular bindings, we would now have to pass the appripriate factories for those same columns. And to support und/redo and string trimming, the undo/redo factory and string trim factory would have to be repeated for *every column*! :-( > - the Javadoc will be easier to write since there is no magic True. Less abstraction == easier to explain. :-) Except that we still need the factories and description objects to support tables, so all that JavaDoc just came back. :-( > - binding is still just one line of code Well, a bug humungeous line of code. With 50% of the code being boilerplate that you have to repeat *each* time you want to bind. :-( And I'm pretty sure that this model doesn't work very well for tables--especially CompositeTable, which needs to be bound recursively and automatically by the framework itself once the top-level bindings are specified. > - no need for a "standard factory" that would make it hard to separate the > framework into several plug-ins (core framework, SWT/JFace support, Beans > support) Due to table bindings, especially CompositeTable, we still need a standard factory interface and standard description object, at minimum, in order to polymorphically bind SWT controls used as cell editors. So in principle, I'm not against refactoring our API to be easier to understand and I think that this is a worthwhile discussion. And I'm not against coming up with a different kind of factory setup, or maybe somehow decoupling the factories from the data binding context's bind method to make the two easier to explain. I think I agree with the underlying sentiment that dbc.bind, as it stands, is trying to do too much in one place. In the end, I believe that this proposal simply moves the configuration complexity out into the application layer without really solving the underlying problem of how to have a nice intentional binding API while allowing for standardized configuration to be centralized. And I'm deeply concerned about how table binding will look under this proposed API.
So to be positive, here are the requirements as I see them: - Make the API more intentional. Make use of dbc.bind() easier to understand. - Have a generic factory so that the binding API can call itself to do more binding when necessary. - Have something we can explain as easily as possible to folks. - Be able to configure behaviors like undo/redo and string trimming in a central location if desired. - Permit developers to repeat configuration for behaviors like undo/redo and string trimming everywhere if that is preferred. Have I fairly expressed everyone's concerns?
One more thought: Maybe a layered approach can solve our dillema?
Short answer: I like Stefan's proposal very much. (Very) Long answer: I wrote this last night, and it turns out that Dave agrees with me more than I thought he would. ;-) Frankly, the current DBC is incomprehensible without lots of discussion with the people watching this bug. Like Stefan, I discovered over time that DBC does something totally different than I initially expected. Dave is right that individual calls are not explicit nor stable enough in their atomic behavior. IMHO, the DBC does both too much and too little. It holds onto too much configuration state -- factories and priorities, metadata types, etc. -- and it is too passive during the actual event-handling/value-updating process. It seems that Dave thinks of context as "stuff I've configured before I bind things", while I think of context as "the ongoing situation that needs to be managed". The way I interpret Stefan's proposal, the DBC becomes a binding runtime manager, but it does not participate in the up-front configuration or factory process. Something else is responsible for giving the DBC two boxes and a BindSpec, aka: "stuff that happens between the boxes" object. Client code that merely responds to high-level events generated at runtime, e.g. to display validation messages, would know only a well-defined API for the active process of propagating changes from one box to another. Advanced clients could listen to a fine-grained BindingEvent API, while simple clients use direct getValidationErrors() and similar. Someone can go work on a really nice alrithm for dependency networks without worrying about where the boxes come from or what exactly the BindSpec does. If all IObservables conform to the same type-opaque getValue/setValue/valueChanged interface, questions like "how do we deal with different kinds of metadata" can be punted closer to the code that generates the metadata. All clients to the algorithm are responsible for generating a representation of "my current state" and a representation of "this just happened to me". That is all the DBC needs to know. The individual IObservables are responsible for handling whatever gets passed to it. The client code has more control over making sure that their factories don't bind an IObservable<IStructuredSelection> with an IObservable<ImageData> ... unless that's what they _want_ to do. Who is the DBC to tell me I can't? ;-) Stefan has a good baseline answer for where the observables come from: domain-specific factories. For client code that knows what type of object is being decorated, they can call nicely atomic methods with staticly typed arguments. Clients that have only partial information, or that want to parameterize the factory call somewhere else, can call progressively more general methods: IObservable observeSelection(Combo combo); OR IObservable observe(Combo combo, String propertyName); OR IObservable observe(Control c, String propertyName); To facilitate meta-factory logic, each domain-specific factory should also supply a standard, universally-generic factory method: /** * Integration method -- when you don't know what to call, use this method. * @param genericInput should be one of the Description classes in org.eclipse.*.mydomain or a MyDomain instance * @return iff the return value is not null, I know how to deal with this input */ IObservable observe(Object genericInput); To help newbies navigate the code, I think an empty Description interface should be marked on the current domain-specific description classes. The dispatching logic in the current factories can stay unchanged, and would still use the RTTI of the input + domain-specific metadata in the Descriptions. The case-specific construction logic gets promoted to staticly-typed and documented API, and basic usage dodges the dispatch overhead. "What does the BindSpec do?" can be answered in a similar fashion. The DBC should not be allowed to separate or cache the individual components of a BindSpec, but should "call-through" the BindSpec each time validation or conversion is needed. This way, client code can inject a fully-dynamic BindSpec or just delegate to simple framework-supplied defaults. Client code would not be required to encapsulate conversion and validation in separate classes, although the default BindSpec might do so. The BindSpec could be created by a factory, use a factory, or be a factory -- the DBC doesn't care. This dodges the whole messy debate in bug 121127. If someone wants to use a factory-choosing strategy of "pick one and write it down", they can call something like: ClassBindSpecFactory.createBindSpec(Class a, Class b); OR EMFBindSpecFactory.createBindSpec(EClass a, EClass b); etc. If they want a no-nonsense default thing that handles primitives and Object.toString(), they can use a general-purpose PrimitiveValidatingBindSpec. If the binding-specific book-keeping is stored in the Binding, and not the BindSpec, there can be a singleton BindSpec for the entire application. Yay efficiency! Yay simplicity! Again, for meta-factory purposes, there should still be an untyped generic interface: /** * Factory for Binding "glue code", such as validators and converters. * If this factory is used in static fashion, a and b can be metadata of some kind, * and the return value should be supplied to DataBindingContext.bind(). If you * want your glue code to be dynamic based on the runtime values of the observables, * use a DynamicBindSpec instead and configure it with one or more of these factories. */ interface BindSpecFactory { BindSpec createBindSpec(Object a, Object b); } Dave can use a BindingUberFactory as gatekeeper and be master of the universe. The UberFactory gets built with these cleanly encapsulated component parts. The untyped signature exists only to provide an integration path. Domain-specific factories would NOT be expected to do anything special with e.g. MyFunkyMetadata objects unless they already know how to deal with that. A ClassBindSpecFactory could either return null or do some well-defined default thing, like createBindSpec(a.getClass(), b.getClass()). There is no magic unless you explicitly ask for it (and commit to the up-front configuration cost). To address Dave's concern about Table bindings: If there is a well-defined way to compose each of the fine-grained participants, there will be a straightforward solution. All software problems can be solved by one more layer of indirection. ;-) In most cases, a TableDiff is just a ValueDiff with an extra layer of metadata to specify the row and column. We could supply a TableBindSpec that unwraps the TableDiff and delegates to the appropriate member of a Map<ColumnDescription,BindSpec>. Similarly, a TableObservable would be responsible for decorating a Table based on a ColumnDescription, and would produce the TableDiffs. All of the discussion above still applies -- e.g. the BindSpec that ultimately drives the behavior can be as simple or as complex as desired. In a future, generics-enabled version of the framework, true static type-checking could be used. Yes, the situation is really this complex, even statically: BindSpec<A<T,V> extends Observable, T<V> extends Diff, V, B<U,X> extends Observable, U extends Diff, x> { void validate(A a, B b, T diff, Errors errState); void validateBack(B b, A a, U diff, Errors errState); ... } TableDiff extends Diff<String> TableObservable extends Observable<TableDiff,String> TableBindSpec<B,U,X> extends BindSpec<TableObservable,TableDiff,String,B,U,X> TableBeanBindSpec<X> extends BindSpec<TableObservable,TableDiff,String,PropertyObservable<x>,ValueDiff<x>,x>
I think this proposal would resolve a bunch of design bugs. For 147515, observables become more opaque for the DBC but more explicit for the domain-specific logic. Stefen gets statically typesafe Observables with no explicit getClass() or Type.class. 146906 gets resolved naturally by the Javadoc and typing on the factory methods. For 121127, the runtime algorithm can ignore the static/dynamic aspect, which will be encapsulated in the BindSpec. 147364 is mostly encapsulated in the DBC, which becomes the runtime manager. 116657 is a minor change in the IObservable interface, or is handled in the DBC. 116465 and 146162 get layered on top of the standard observables, if clients are willing to accept more dependencies. 147128 can be a domain-specific tweak that uses a new runtime feature. I could go on and on -- most of the bugs get easier to divide and conquer. If we have consensus, we can all go off and start conquering our favorite bits. :-)
Peter, thanks for your detailed comments. My first knee-jerk reaction to your proposal is: 1) I think you're right that it would make the client-level API cleaner and simpler, but at the expense of making the extension API more complicated and difficult to use. Clients will need to only know about the static factories that they use. But extenders will have to both create a static factory *and* create a generic factory. 2) Our current data binding library tries to use one API to satisfy three concerns, all at once: a) Defining and implementing a data flow / constraint graph. b) Mapping database semantics onto any kind of Java object (POJO, EMF, etc). Purpose: to help interface Java objects with O/R mappers, object databases, or other object persistence layers. c) Binding domain objects in the data flow graph to user interface controls. This has yielded a cartesian product explosion of concerns all across the API. 3) I think that Peter's dualality of factories is a symptom of this problem. Perhaps we could look at separating these concerns into layers that work together somehow and improve the API simplicity even more?
In my mind, 2c is a strict subset of 2a. There will be many domain-specific constraints and data flows, and a major purpose of a binding framework is to provide structure to that problem. The core algorithms are very complex but general, so they can and should be encapsulated. Precisely what do you mean by "database semantics"? I don't see evidence of ACID transactions or query support here, and I genuinely hope it stays that way. ORM tools own these problems already. I don't grok the "duality of factories" reference. I disagree about your conclusions in 1). The "static factories" are just a human-friendly face to the logic that would be there anyway. Instead of burying the code inside an untyped signature, the framework API presents itself as smaller, predictable methods. Users can make an easy inference that if a class can do something piecemeal, it probably can do it through the generic signature, too. If a class can't do something piecemeal, it's probably not going to do it though the generic signature, either. If client code wants to contribute to the process, it only has to abide by the generic contract. It can make its own decisions about what to publish as API and what to keep internal. This imposes no new requirements on extensions.
(In reply to comment #21) > In my mind, 2c is a strict subset of 2a. There will be many domain-specific > constraints and data flows, and a major purpose of a binding framework is to > provide structure to that problem. The core algorithms are very complex but > general, so they can and should be encapsulated. > > Precisely what do you mean by "database semantics"? I don't see evidence of > ACID transactions or query support here, and I genuinely hope it stays that > way. ORM tools own these problems already. It's 1:03 AM my time, so I hope this is mildly coherent. :-) On the UI side, you have the concept of a value that is in the process of being changed, but isn't yet complete. On the database side, you have the concept of a record that is in the process of being changed but could be rolled back or committed. Mapping these ideas to each other is nontrivial and we currently don't solve the problem well. Suppose you're editing some objects that are mapped through Hibernate to database tables. You leave the Hiberrnate session open so that you can edit objects that have a live connection to the database. The question: When do you flush the Hibernate session? The API currently doesn't address this question at all. At my current client, we have worked around this deficiency by listening to binding events and flushing the Hibernate session whenever a target-to-model copy completes. But this only works for us because we are using a replicated database, so we don't have to worry about generating too much network traffic through having too many transactions. In a true multiuser environment, our solution would not scale. So you have to deal with the database concerns somewhere. The database concern is summarized as answering the question of what user interface gestures demark a database transaction. There needs to be API so that the database transaction can be committed or rolled back at the appropriate time. I find it interesting to note that Microsoft's and Borland's solution to this problem is to flush the transaction when the cursor leaves the database row. So multiple fields can be dirty in the transaction at once. But normally not multiple rows. This solution dates back to the DOS 4GL environments in the late 80s, and seems as good an answer today as anything else I can think of. In the context of OO data binding, the analog to leaving the database row is leaving the parent object that you are editing. So if you have a table of orders, leaving an order row is what triggers a flush. If you have 1-M relationships, then you can have nested transactions active if your underlying O-R mapper supports it, and this really needs to be supported in the API too. > I don't grok the "duality of factories" reference. My understanding is that we are proposing to have a human-friendly API that consists of classes with static methods in them. This is one API. But we also have this requirement that data binding be able to consume itself. So you suggested that we have a generic binding interface as well that more sophisticated bindings could use to perform recursive or nested bindings. I'm not necessarily 100% against this. But I do feel it is important that we recognize that this is what we are doing. Here's where I'm coming from: I'm generally supportive of the ideas that are being proposed, but I'm skeptical that this will yield a highly-usable API when these ideas are applied to common but complex binding situations. But I'd rather not discuss this in theory any more either; I feel that we will all know what we like a lot better when we see real code rather than just from debating in the abstract. I would like to propose that we spike a data binding API for three things: 1) TableViewer including Text and Combo cell editors. 2) CompositeTable with Text and Combo objects in the row object. 3) A Text bound to an int property. The examples should support undo/redo through Eclipse's undo/redo mechanism and should trim empty strings to nulls when values are copied from the target back to the model. While we're at it, the API should support SWT controls being in a dirty (changed but unsaved) state, and an object also being in a dirty (changed but unsaved) state. An attempt should be made to describe how one would hook up an event handler to the API to save an object when the focus leaves the object. And when I say "spike" I mean write working API. That API can be written on top of the current API in order to make it work more easily. The reason I have proposed these specific cases is because I think that these cover most of the corner cases that one runs across in real life. In particular I want to know two things: 1) What clients of the API will have to work with. 2) What extenders of the API will have to work with.
Just to clarify my intent, in case I need to: I'm not proposing a prototype/spike to shoot down others' suggestions but rather as a vehicle to help us come to a concrete agreement and drive consensus about what we need to do. No politics going on here. :-)
A prototype dry run sounds great to me. I'd be happy to take on some of the load, if the committers want to farm a chunk out to me. My app doesn't have in-line editable tables at the moment, so it would have to be a form-like view. I still think that the question of transaction granularity is orthogonal to data binding. I'm a firm believer in the "view model" layer to mediate such questions -- i.e. what I'm binding the view _to_, not the binding itself. I personally would not want to build assumptions about transactions into my binding setup logic, since I rely on other people to tweak server scalability. I honestly can't say right now what would be best for my app. If this is useful for others, though, don't let me hold you back -- just let me opt-out. :-)
(In reply to comment #22) > I would like to propose that we spike a data binding API for three things: > > 1) TableViewer including Text and Combo cell editors. I'll take TableViewer as I've implemented the API in bug 144260. I'll just need to expand the support to the Combo CellEditor. The only caveat is that I'll be on vacation next week so if I don't get to it this weekend it'll be another week before I get to it.
> I'm generally supportive of the ideas that are being proposed, but > I'm skeptical that this will yield a highly-usable API when these > ideas are applied to common but complex binding situations. FYI, in my app I'm using these patterns extensively, so can provide plenty of examples if you have some concrete use-cases. > dbc.bind(SwtObservables.getText(myTextbox), > UndoRedoObservables.getUndoRedo( > StringTrimObservables.getStringTrim( > (BeanObservables.getBean(myBean, "propertyId")))); If the app does this all over the place, then the developer could write: dbc.bind(SwtObservables.getText(myTextbox), MyAppBeanObservables.getBean(myBean, "propertyId")); ...where MyAppBeanObservables provides the static helper that creates the chain of wrappers that the app uses all over the place. The above comments have disussed the management of transactions, undo, and dirty state. This is good stuff... but I'm not sure if we need the framework to solve these problems for us, or merely to to provide the glue (reusable binding and observable interfaces) that will let us solve these problems in isolation and reuse the solutions easily. Dave: you've mentioned TableViewer as an example of where extra abstractions on bindings are necessary, but I suspect that your use of tables is very different from mine since my tables use concrete bindings without a problem. I use my own custom table-viewer-like class that wraps (but does not extend) the JFace one. It takes its input as an IObservableSet that provides the rows, along with a transformation that generates each column. Here's some pseudocode: CustomTable myTable = new CustomTable(parent); myTable.setInput(someObservableSetOfCustomers); CTColumn column1 = new CTColumn(myTable, "Name", new Transform() { Object convert(Object row) { return ((ICustomer)row).getName(); } }); CTColumn column2 = new CTColumn(myTable, "Balance", new Transform() { Object convert(Object row) { return ((ICustomer)row).getBalance(); } }); I never need to create observables or bindings by introspection or via an abstract factory that takes Property objects. Since I know what type of object to expect in each column, I could just supply the correct observable (or a factory for it) directly. I suspect you may be doing something more sophisticated than I am. Are you writing something like a spreadsheet program where you can have a variable number of columns containing mixed-type data? I'd like to understand why hard-coded column bindings wouldn't work.
Stefan, you've 95% convinced me. :-)cerns: Here's my remaining concerns: 1) Bug 147128. I think we still need more explicit master-detail associations. I believe the reason this is relevent will be come apparent later. 2) CompositeTable binding. This answers your question directly about what I'm still concerned about with table binding. CompositeTable works the following way: a) Create a custom SWT control extending Composite that contains the SWT controls that represent each column in your table. (Assume a GridLayout on this Composite for now.) b) Create an instance of your custom SWT control inside a CompositeTable instance and set CompositeTable's properties to indicate number of rows in the underlying collection, etc. Add your event handlers to populate CompositeTable with data. (CompositeTable is fully virtual.) CompositeTable will use reflection to duplicate your prototype row object for as many rows as are visible. It will call your event handlers to populate those row objects with data. The event handler receives: i) The row object to fill with data. ii) The offset (a 0-based long) into your collection of the object to use for filling the object. Binding a CompositeTable requires: i) Binding the collection using a LazyListBinding to the CompositeTable itself. ii) Registering a *generic* event handler that can bind each control in the row object to a property of the object inside the list. Part (ii) above requires that the CompositeTable binding's event handler be able to generically bind anything to anything. It does not know what kind of SWT control it will receive: built-in or custom. It does not know what kind of object or property it will have to bind that control to. So CompositeTable binding is really powerful because you get in-place editing for free and you get all the field-level validations for free. But it also is really general and pretty much requires all of the meta-factories that we currently have in data binding in order to work right. Or maybe you've thought of a way around this that I haven't seen yet. Lastly, if one of the controls in the row object is a Combo, issue (a) becomes a potential problem in the API.
Andy Maleh suggests turning DBC into an implementation of the Builder pattern: BindingBuilder builder; builder.addTarget(text); builder.addTarget(label); builder.addModel(person, "name"); Binding binding = builder.createBinding();
(In reply to comment #28) > Andy Maleh suggests turning DBC into an implementation of the Builder pattern: > > BindingBuilder builder; > builder.addTarget(text); > builder.addTarget(label); > builder.addModel(person, "name"); > Binding binding = builder.createBinding(); > In comment 2 Stefan's original proposal was to remove the API that creates observable from the DBC so that the DBC... > "Manages the lifecycle of a set of bindings, and collects their validation messages. " What problem would changing DBC to a builder pattern solve? Also isn't it a builder today just with a slightly different API than the proposed?
[I posted a comment here last Friday, but it disappeared for some reason.] Hello, my name is Andy Maleh and I am currently involved in an Eclipse RCP project with David Orme. I was brainstorming a few ideas with Dave on simplifying data binding and making it more readable and intentional as this has been a big problem for our team in maintaining data binding code. It often seems too cryptic a few weeks after it is written. So, I suggested the builder pattern as it clearly identifies elements when building a binding in comparison to passing parameters to the DBC bind method. This is orthogonal to Stephan's remarks, and the builder could probably be implemented as a facade on top of data binding's architecture. The facade would be used by those who would rather write a few more lines of code that are easier to read during maintenance. Still, that was a suggestion amid a brainstorming session, so feel free to improve upon it or add other ideas. Best regards, Andy Maleh p.s. thanks to all the people that contributed to data binding. It has been very helpful in our project.
Hi Andy. Thanks for the comment. My questions revolves around the API that Dave posted. It looks like you have a need to bind multiple descriptions at the same time. I just wanted to clarify your use case and how often this occurs. I would assume it would occur infrequently. Also in what Dave posted it looks like the API is still untyped so all add methods accept an Object. Is this correct?
I would very much like to see us make a decision on this issue so that we can proceed with development. IMHO, the safest solution is for us to start by breaking apart DBC and writing the strongly-typed APIs. This will simplify a lot of code, and will be useful to all of us - even if it is only a partial solution for those of us who ultimately want the abstract factories. Those of us who need abstract factories can still implement their own on top of the strongly-typed framework. If we find that enough of us are doing this, we can always bring them back as an additive change. When and if we bring them back, we will have more than one pattern to consider and the patterns we have will have been well tested. In the interest of ending the discussion and proceeding with development, could we agree to put this issue to a vote (or otherwise decide this matter one way or the other)? Proposal 1: Break up DBC as proposed, remove the abstract factories, and develop concrete factories. Abstract factories will be implemented privately by those who need them, and can always be brought back later once the framework is in heavier use and we have a chance to see a wider set of use-cases. Proposal 2: Leave DBC with its current responsibilities. Reduce the boilerplate in simple client code and examples by providing a default DBC. Concrete factories will be implemented privately by those who need them.
(In reply to comment #32) > Proposal 1: Break up DBC as proposed, remove the abstract factories, and > develop concrete factories. +1 from me for Proposal 1. However, we should look at the table scenarios Dave is concerned about. I will have some time for data binding work next week - if anyone (Dave?) would like to chat with me about this, I am available.
(In reply to comment #33) > (In reply to comment #32) > > Proposal 1: Break up DBC as proposed, remove the abstract factories, and > > develop concrete factories. > > +1 from me for Proposal 1. However, we should look at the table scenarios Dave > is concerned about. I will have some time for data binding work next week - if > anyone (Dave?) would like to chat with me about this, I am available. Two thoughts: 1) I'm currently in the process of writing LazyListBinding support for CompositeTable in my spare time to see if we can accommodate proposal 1 with a reasonable API for complicaated nested binding scenarios. Boris, please call me when you get a chance so we can review the LazyListBinding API and friends. 2) If this works out okay, then I'm fine with proposal #1. However, if it turns out that CompositeTable simply cannot be bound reasonably with this API, then I think we'll need to look for a 3rd solution that we haven't thought of yet. However, my hope is that I'll be able to find some variation of the strategy pattern that will make binding with Stefan's proposed API work cleanly with CompositeTable. In any case, I'll report on my findings.
I have results on CompositeTable binding with a Stefan-friendly API. ;-) Here's my bindGUI method: private void bindGUI(Shell shell) { DataBindingContext bindingContext = new DataBindingFactory1().createContext(shell); IRowBinder rowBinder = new IRowBinder() { public void bindRow(DataBindingContext context, Control row, Object object) { Row rowObj = (Row) row; Person person = (Person) object; context.bind(rowObj.name, new Property(person, "name"), null); context.bind(rowObj.address, new Property(person, "address"), null); context.bind(rowObj.city, new Property(person, "city"), null); context.bind(rowObj.state, new Property(person, "state"), null); } }; CompositeTableObservableLazyDataRequestor tableObservable = new CompositeTableObservableLazyDataRequestor(bindingContext, table, rowBinder); bindingContext.bind(tableObservable, (IObservable)personList, null); } As you can see, the code uses a mixture of direct observable instantiation (tableObservable, personList) and factory-based binding inside the strategy implementation. But there's no reason this strategy implementation couldn't use Stefan's style of factories. I think the resulting code is more intentional than what we had, and would be significantly cleaned up over what I have above when it is migrated completely to Stefan's API proposal. There's also the odd thing that I had to explicitly typecast personList (which is a WritableList) to an IObservable or the compiler called the wrong overloaded version of dbc#bind. This also would be fixed by having explicit bind methods for pairs of ObservableValue objects, ObservableList/Set, and lazy observables. And the compiler would automatically check that compatible observables were being bound. Nice. So my main concern now is that when we change the code base, the current factories get migrated into the examples plugin so that we will have a migration path for our code base. Also, we are within 1 month of shipment for our application, so anyone who wants to hack on this, please give me a heads-up first so I can branch the tree. We obviously can't sustain a significant API change right now.
(In reply to comment #31) > Hi Andy. Thanks for the comment. My questions revolves around the API that > Dave posted. It looks like you have a need to bind multiple descriptions at > the same time. I just wanted to clarify your use case and how often this > occurs. I would assume it would occur infrequently. Also in what Dave posted > it looks like the API is still untyped so all add methods accept an Object. Is > this correct? > Brad, Binding multiple descriptions does occur every once in a while; usually when binding enablement of multiple controls to selection of a checkbox. It was added in the example as a brainstorming idea, but the main concern was readability, not supporting multiple bindings. As for the API being untyped... it can be made typed as follows: BindingBuilder builder = new BindingBuilder(); builder.addTarget(SwtObservables.getText(myTextBox)); builder.addModel(BeanObservables.getBean(person), "name"); builder.createBinding(); While the API above may make code easier to understand and maintain, it seems too verbose to me for simple binding tasks. Another way to make data-binding code simpler is as follows: BoundText myTextBox = new BoundText(parent, SWT.BORDER, person, "name"); BoundLabel myLabel = new BoundLabel(parent, SWT.NONE, person, "age"); or building on Stefan's idea: Text myTextBox = SwtBindingFactory.createText(parent, SWT.BORDER, person, "name"); Label myLabel = SwtBindingFactory.createLabel(label, SWT.NONE, person, "age"); SwtBindingFactory would be a higher layer of abstraction that uses proposed SwtObservables.getX methods underneath. Any other ideas??? Best regards, Andy
(In reply to comment #36) > Binding multiple descriptions does occur every once in a while; usually when > binding > > enablement of multiple controls to selection of a checkbox. It was added in the > example as a > > brainstorming idea, but the main concern was readability, not supporting > multiple bindings. I guess this can be handled multiple ways, depending upon your needs. If the controls that need to be enabled are all in a single Composite enablement could be bound to the Composite's enabled property (or even some other manager type object) and when changed a recursive method could apply the enablement to all children. In that case there wouldn't be the need to bind every control. But if the children can't be sectioned off in their own Composite I can see it getting a little tedious. > As for the API being untyped... it can be made typed as follows: > > BindingBuilder builder = new BindingBuilder(); > builder.addTarget(SwtObservables.getText(myTextBox)); > builder.addModel(BeanObservables.getBean(person), "name"); > builder.createBinding(); So where does the DBC come in? Has the DBC become BindingBuilder? If the BindingBuilder accepted a DBC and the DBC still handles binding observables (which all depends upon what happens with the rest of this bug), this could then be simple to implement in consumer code. > Another way to make data-binding code simpler is as follows: > BoundText myTextBox = new BoundText(parent, SWT.BORDER, person, "name"); > BoundLabel myLabel = new BoundLabel(parent, SWT.NONE, person, "age"); > > or building on Stefan's idea: > Text myTextBox = SwtBindingFactory.createText(parent, SWT.BORDER, person, > "name"); > Label myLabel = SwtBindingFactory.createLabel(label, SWT.NONE, person, "age"); IMHO I don't think databinding is the place for this. This would be making databinding an SWT factory as well and I think these are 2 different concepts. If we did this I would expect consumers would want to be able to customize the factory to enforce a standard look and feel in their application. For example, they might want to ensure that all Text widgets have a border as enforcing style is a common task of a widget factory. If we did that we'd need to have API to allow for customization of our factory which is this issue all over again. I think if people want an SWT factory it needs to be separate. Consumers could wrap the SWTObservables factory in their own factory to account for their needs with their own API, documentation, etc.. To me that's a more scalable approach to this. At this point I'm indifferent on the BindingBuilder concept as long as it fits in with the rest. Plus if it could be created by a consumer in their own code and not part of the databinding API I think that's even better. I'm curious as to what is going on with the rest of the proposal. Guys, is progress being made? What are the next steps? I'm pretty busy with bug 146162 and will be for a while but if there's anything I can do to help let me know.
We had a team meeting here at my client to talk about what could be done to make the data binding API more intentional. We generally agreed that Stefan's proposal makes sense, and we felt that the following refinement would provide additional improvement. Refinement of Stefan's idea: If we're constructing Observables directly, we can reuse SWT's API; there is no need to invent new API. // To make an Observable value on an SWT control: Text text = new Text(parent, SWT.BORDER); // IObservableValue.DEFAULT == FocusOut | ESC | CR | LF IObservableValue observable = new SWTObservableValue(text, IObservable.DEFAULT); // To make an Observable list on an SWT control: Table table = new Table(parent, SWT.BORDER); IObservableList observable = new SWTObservableList(table, IObservable.NULL); // To make an Observable value on a table selection: Table table = new Table(...); IObservableValue selection = new SWTObservableSelection(table, IObservable.NULL); // To make an Observable list on a JFace viewer: Table table = new Table(parent, SWT.BORDER); TableViewer viewer = new TableViewer(table); IObservableList observable = new ViewersObservableList(viewer, IObservable.NULL); Putting it all together like Stefan's example, a complete binding would look like: dbc.bind(new SWTObservable(text, IObservable.NULL), new BeanObservable(person, "firstName"), null); Advantages: * Inheritance works better with constructors than with static methods on factories. * As little new API as possible.
Are you planning to support annotations ? I'm building a enterprise application based on Eclipse RCP. I use the following syntax for "inject" and "outject" data from a bean to a field : @In(el="person.name") CLabel name; (data are stored in a context) I find it very convenient and simple.
(In reply to comment #39) > Are you planning to support annotations ? No, because we would like JFace data binding to run on J2ME Foundation 1.0 class libraries (a subset of the J2SE 1.3 class libraries) just like JFace proper. > I use the following syntax for "inject" and "outject" data from a bean to a > field : > > @In(el="person.name") > CLabel name; Interesting. Can I read more about this? Is it described in more detail somewhere?
(In reply to comment #39) > @In(el="person.name") > CLabel name; I don't know how this works in detail, but I would suspect that you could write a layer on top of the data binding framework that does just this.
I will try to put it on sourceforge in a near future. Basic ideas come from JBoss Seam : http://docs.jboss.com/seam/1.0.0.GA/reference/en/html/concepts.html (look at the bijection section). I want to simply development of enterprise application based on Eclipse RCP. I want it to be accessible to corporate developers. In that way I can integrate "Struts developers" on my project. (In reply to comment #40) > (In reply to comment #39) > > Are you planning to support annotations ? > > No, because we would like JFace data binding to run on J2ME Foundation 1.0 > class libraries (a subset of the J2SE 1.3 class libraries) just like JFace > proper. > > > I use the following syntax for "inject" and "outject" data from a bean to a > > field : > > > > @In(el="person.name") > > CLabel name; > > Interesting. Can I read more about this? Is it described in more detail > somewhere? >
This is an interesting idea. We are currently reading JPA annotations to create IValidators like StringLengthValidator and required decorations for DecoratedFields. So for example the binding of a field who's model has an annotation of @Column(nullable=false,length=5) we automatically add a new StringLengthValidator(5).
1. Can we move the annotations idea to a separate bug? It is orthogonal and should be tracked independently. 2. In reference to #38: I'm in favor of separating a) the widget construction from b) the Observable construction. Brad makes a good point in #37 that these things need to be decoupled so that clients can intervene between them. However, I'm leary of using constructors instead of factory methods for b). It is much more difficult to reimplement/extend the factory logic because this API makes a bytecode-level commitment to the exact return type. I'd prefer: Text text = new Text(parent, SWT.BORDER); IObservableValue observable = SWTObservableValue.observe(text, IObservable.DEFAULT); In this API, the framework retains flexibility to use a polymorphic SWTObservableValue, if necessary. 3. Riffing off of the micro-snippet above, we can debate the exact signatures of the factory methods. Design space includes: a) Ultra-minimal parameter lists. // Observe the default property of the param with default bindSpec IObservableValue textTextObservable = SWTObservableValue.observeText(text); // ... or tweak the defaults IObservableValue textEnabledObservable = SWTObservableValue.observeText(text, SWTObservableValue.ENABLED); b) Method overloading. IObservableValue textTextObservable = SWTObservableValue.observe(text); IObservableValue buttonEnabledObservable = SWTObservableValue.observe(button); c) Single factory class, multiple return types. IObservableValue textTextObservable = SWTObservable.observe(text); IObservableList tableContentsObservable = SWTObservable.observe(table); d) Loosely typed uber-factory method. Control mysteryControl = leapOfFaith(); IObservable mumbleMumbleObservable = SWTObservable.observe(mysteryControl); Note that making this method available can lead to counter-intuitive dispatching behavior. The first observe() below would call the uber-factory, but the second would call a more specific overload. Control mysteryControl = leapOfFaith(); IObservable mumbleMumbleObservable = SWTObservable.observe(mysteryControl); // The truth is revealed! Text iAmText = (Text) mysteryControl; IObservableValue textTextObservable = SWTObservable.observe(iAmText); e) Singleton factory vs. factory bean vs. static factory. SWTObservable observer = // singleton // SWTObservable.getInstance(); // factory bean new SWTObservable().setBlah(blah); // Not a static call anymore! IObservableValue textTextObservable = observer.observe(text); 4. My $0.02 on the alternatives above is to favor the API client over the API extendor in all things, but provide a path to reduce framework pain where possible. Concretely, this means yes to a), b), c) and maybe d). Someone with more optimization and/or API design experience should decide e), but I lean towards the factory bean approach for maximum flexibility. SWTObservable would be a pretty large API, with 3*n methods, where n == number of distinct SWT widget classes. It should still be easy to extend if it's consistent about forwarding methods. E.g. observe(Text) forwards to observe(Text,String) which fowards to observe(Text,String,int), etc. If we expose an uber-factory method, it should forward to something like unhandledInput(Control) and throw an exception by default. * both clients and extendors get a focussed point of control for SWT binding setup * clients have flexibility to be more or less specific about their intentions * extendors have flexibility to implement polymorphic IObservables (or not)
P.S. For clarity, the factory should probably be named SWTObservableFactory or SWTObserver and be distinct from SWTObservable. SWTObservable implements IObservable and is the root of the type hierarchy for the return values of SWTObserver.observe(). SWTObserver would not implement IObservable.
Interesting ideas Peter Centgraf. To build on your idea further and achieve ultimate flexibility, we can program a configurable factory class that would store a map of strategies for manufacturing SWT observables. Each strategy is a 'micro' observable factory that is responsible for one type of control only. Here are code examples (note that code is incomplete and imperfect as it is written mainly for demonstration purposes): /** observable value factory interface */ public interface IObservableValueFactory { public IObservableValue observe(Control c); } /** Code snippets of envisioned configurable factory class */ public class SWTObservables implements IObservableValueFactory { ... public IObservableValue observe(Control c) { return observableFactoryMap.get(c.getClass()).observe(c); } ... public void addValueObservableFactory(Class controlClass, IObservableValueFactory observableFactory) { observableFactoryMap.put(controlClass, observableFactory); } ... more methods } /** 'micro' control observable factory example */ public class TextObservableFactory implements IObservableValueFactory { public IObservableValue observe(Control c) { Text text = (Text)c; return new TextObservable(text); } } observableFactoryMap maps IObservableValueFactory objects to SWT classes. This enables us to flexibly configure extra observable factories whenever we need to. For example: SWTObservables.addValueObservableFactory(MysteryWidget.class, mysteryWidgetObservableFactory); Likewise, lists are handled with IObservableListFactory interface similar to IObservableValueFactory. SWTObservables may also extend that interface. Details can be worked out further, but this is the general idea. It can also be easily adapted to David Orme's idea of using constructors instead of factory methods, which is supposed to allow easy support of data-binding by GUI builders (Visual Editor, WindowBuilder, etc...) to my knowledge. Regards, Andy p.s. By the way, under bug 158398, I posted an idea to simplify the use of data-binding for simple common use cases: https://bugs.eclipse.org/bugs/show_bug.cgi?id=158398 Feel free to contribute your opinion/feedback/suggestions.
Andy, thanks for the comments. I disagree with your proposal, though. :-) Pro: Allows new SWT widgets to be handled without changing public API. Con: Public API has no meaning. I envision a very focused class. SWTObservables would not be used to observe custom or third-party SWT widgets. External client code would have to provide the real logic anyway. Why ask SWTObservables to manage a process in which it has no meaningful contribution? Clients would be free to build a meta-factory around SWTObservables and their own custom factory logic if they want, but the databinding framework IMHO should not dictate the organization of that code. A change to the SWT type domain is an API change for clients, and I see no reason that this should be different for data binding. On the contrary, it makes perfect sense that the API should make new capabilities public and obvious. E.g. if SWT adds a new Calendar widget in 3.3, a human should be able to read the SWTObservables JavaDoc and immediately know if they can observe Calendars or not. Better yet, the JavaDoc can clearly describe which Calendar property will be observed. Pro: Factory logic is broken into smaller, more narrowly-defined pieces. Con: Dynamic typing instead of static typing. A major goal of this (evolving) proposal is to make the behavior of the framework easier to understand. One reason this is problematic now is because there is no type information on the parameters for API methods. This means that method dispatch is not fully determined until runtime, so it is impossible for either human or machine to trace an execution path statically. Any kind of dynamic indirection based on runtime type has this problem. Plus, casts are ugly and scary. I don't want to make it standard practice to require them. Generics are not available to us because of compatibility issues. Con: Proliferation of classes Your design implies the creation of at least n+1 concrete classes and m interfaces, where n = number of SWT control classes and m = number of IObservable-derived interfaces. As of this writing, n=14 (including swt.custom) and m=5 => 20 new types. It would be reasonable (but inadvisable) to implement separate concrete classes for each valid combination of control and IObservable type, which would perhaps double the number of new API types and set an upper limit of n*m+m. Thats a lot of API surface area for clients to deal with. In contrast, I'm proposing 1 new type that would replace an existing type (SWTObservableFactory) for a net API growth of zero.
I may have not made the approach clear enough as there seems to be a slight misunderstanding. The user only has to know about SWTObservables, which completely hides the proliferation of classes. The approach I suggested is in harmony with yours and builds on it. So, the syntax remains as you suggested: Control mysteryWidget = leapOfFaith(); IObservableValue mysteryWidgetObservable = SWTObservables.observe(mysteryWidget); I was talking more about the design of SWTObservables to accomplish that simplicity, but not sacrifice flexibility. Pros: -A single point of contact with the API to create SWT observables by calling SWTObservables.observe -Instead of having a getXXX(control) method on SWTObservables per SWT control, observable factories for basic SWT controls are pre-configured in SWTObservables so that users only has to use the observe method -To keep the API simple when binding new widgets or controls, users may configure new SWT Observable Factories within SWTObservables to enable creating observables for new widgets without disrupting API syntax what-so-ever, thus keep using SWTObservables. Cons: -An observable factory class per control Note that this is different from how that was done in the past with regards to DataBindingContext. Those factories are now configurable at the leve of SWTObservables, not DataBindingContext, so the API remains intentional. And, as I said before, the approach may be completely adapted to what David Orme was suggesting where Data-binding syntax is: dbc.bind(new SWTObservable(mysteryWidget), new BeanObservable(model, property)); SWTObservables is changed to now SWTObservable with the following implementation: public class SWTObservable implements IObservableValue { private IObservable wrappedObservable; private Map observableFactoryMap = new HashMap(); //wish we can use generics public SWTObservable(Control c) { wrappedObservable = observableFactoryMap.get(c.getClass()).observe(c); } public Object getValue() { return wrappedObservable.getValue(); } public Object getValueType() { return wrappedObservable.getValueType(); } ... more delegation to wrappedObservable methods /** Optional method to allow ultimate flexibility */ public void addValueObservableFactory(Class controlClass, IObservableValueFactory observableFactory) { observableFactoryMap.put(controlClass, observableFactory); } ... more methods } So, SWTObservable acts as a wrapper around the true IObservableValue that is observing the control, shielding the user from complexity, and delegating calls to the wrapped IObservableValue.
Hmmmm... Adding to what both Andy and Peter have said, I'd like to make an observation: The following two code snippets achieve exactly the same thing with exactly the same semantics from the point of view of an API consumer: 1) public SWTObservableValue(Text text) { ... } Text t = new Text(parent, style); IObservableValue v = new SWTObservableValue(t); 2) class SWTObservableValue { public static observe(Text t) {... } } Text t = new Text(parent, style); IObservableValue v = SWTObservableValue.observe(t); So, the difference between what Peter and I have suggested ultimately is purely stylistic. In other words, do we prefer to use factory methods to construct things or do we prefer to use constructors? Since Eclipse as a whole nearly always uses constructors to construct things, I think we should make that API choice unless there is some way it's severely broken and we need to fix it.
(In reply to comment #49) > So, the difference between what Peter and I have suggested ultimately is purely > stylistic. In other words, do we prefer to use factory methods to construct > things or do we prefer to use constructors? I am probably repeating something that Peter already wrote, but I believe from an API standpoint we prefer factory methods over constructors because it gives us flexibility as to which concrete class implements the observable behaviour.
The paradigm Dave proposed is a factory that looks like a wrapper. "SWTObservables.observe(control)" simply becomes "new SWTObservable(control)" At the bottom of comment #48 , I wrote an example of how Dave's idea would be implemented. It is really just a stylistic variation on the factory method implementation in comment #46. Although this is subtle to note, there is really little difference between both approaches except the style. They both give us flexibility as to which concrete class implements the observable behaviour. SWTObservable wraps around the concrete observable object whereas SWTObservables just spits it out. The code examples in #46 and #48 should hopefully make that clear.
(In reply to comment #50) > I am probably repeating something that Peter already wrote, but I believe from > an API standpoint we prefer factory methods over constructors because it gives > us flexibility as to which concrete class implements the observable behaviour. > Yes, please don't force developers to know the concrete classes that implement the observables! You could then even keep the concrete classes internal or provisional and only make the factories public. But if GUI builders work better with constructors (wouldn't they work just as well with static factories?), you could also make the concrete classes public, but still advise the use of factories for handwritten code. (In reply to comment #51) >"SWTObservables.observe(control)" simply becomes "new SWTObservable(control)" I fail to see how "new SWTObservable(control)" that in turn creates and wraps the real observable is better in any way than either a factory method or "new ActualSWTObervableImplementation(control)". I don't think this is just a style question.
(In reply to comment #52) > (In reply to comment #50) > > I am probably repeating something that Peter already wrote, but I believe from > > an API standpoint we prefer factory methods over constructors because it gives > > us flexibility as to which concrete class implements the observable behaviour. > > Yes, please don't force developers to know the concrete classes that implement > the observables! With my proposal, developers would not have to know the concrete classes that implement the observables. It could be implemented something like this: public SWTObservableValue(Text text) { delegate = new TextObservableValue(text); } public SWTObservableValue(Label label) { delegate = new LabelObservableValue(label); } and then all of the methods implementing IObservableValue would just delegate to 'delegate'. Extending this to support your own project's controls plus SWT is easy. Just subclass SWTObservableValue and add your own constructors. With static factory methods, you have to know the class that originally defined the factory. With subclassing, you can just always use your subclass because you will automatically inherit all of the SWT functionality. > You could then even keep the concrete classes internal or > provisional and only make the factories public. Yes. > But if GUI builders work better with constructors (wouldn't they work just as > well with static factories?), you could also make the concrete classes public, > but still advise the use of factories for handwritten code. If a constructor signature for SWTObservable is: public SWTObservableValue(Control control, int style) {...} you can use it *today* in Visual Editor with *zero* modifications to either Data Binding or VE. > (In reply to comment #51) > >"SWTObservables.observe(control)" simply becomes "new SWTObservable(control)" > > I fail to see how "new SWTObservable(control)" that in turn creates and wraps > the real observable is better in any way than either a factory method or "new > ActualSWTObervableImplementation(control)". I don't think this is just a style > question. I think I just explained how the two can be equivalent? As to why constructors are better, in addition to the extensibility by subclassing argument, people will be coming to JFace data binding from either straight SWT or RCP. They're used to creating SWT controls or JFace control wrappers using: Table table = new Table(parentComposite, style); TableViewer viewer = new TableViewer(table); IObservableList observableList = new ViewerObservableList(viewer); Oh--that last one isn't a JFace API (yet). But maybe it should be. ;-)
(In reply to comment #53) > public SWTObservableValue(Control control, int style) {...} My main issue with this, other than I don't like 20 constructors in a class, is if a consumer was to overload the constructor and created their own style future updates to data binding could break their code. Styles only work if you have full control over them. Plus this would sometimes be specifying a style and others it would be specifying the attribute, like enabled, to observe. Styles are difficult and they make somewhat ambiguous API. Static factory methods allow you to have more explicit names that aren't tied to the class name and I think that's a virtue of the current approach. It allows the consumer to easily state that they want to bind to the enabled property or to just create an observable for the text. SWT has done a good job of maintaining styles but it's a difficult thing to do and for something as dynamic and wide open as the attribute to bind to and customization of an observable I think it would be a risky path to take, especially for a 1.0 release. But to jump back a few comments I am a little confused about comment 38. It said that the API needed to be more intentional. Was your discussion over the old API (DBC and factories) or the current SWTObservables implementation? I'm speaking of methods like... SWTObservables.getText(Text text, int event) To me this is intentional and there's no ambiguity as to what is being bound to and what the outcome will be.
"My main issue with this, other than I don't like 20 constructors in a class" No need for 20 constructors or inheritance when following the implementation at the bottom of Comment #48. One constructor suffices and the API can be easily extended (through addValueObservableFactory method) due to use of delegation/strategy patterns. In any case, if we don't want the constructor approach, and prefer the factory method approach instead, what is your opinion of the highly configurable approach outlined in Comment #46? Please read #46 and #48 in detail if you haven't done so to get a full understanding (note that #48 has some extra line breaks). Idea contributions to improve the approach are greatly appreciated. Remember though, the code was written for demonstration purposes and is not complete or flawless, so it is better to focus on the big picture for now.
Changing to a slightly different topic, here is my response to an old post by Brad: > As for the API being untyped... it can be made typed as follows: > > BindingBuilder builder = new BindingBuilder(); > builder.addTarget(SwtObservables.getText(myTextBox)); > builder.addModel(BeanObservables.getBean(person), "name"); > builder.createBinding(); "So where does the DBC come in? Has the DBC become BindingBuilder? If the BindingBuilder accepted a DBC and the DBC still handles binding observables (which all depends upon what happens with the rest of this bug), this could then be simple to implement in consumer code." Yes, you are correct, DBC becomes BindingBuilder. Main benefit of builder is making syntax more readable. Syntax changes from: dbc.bind(SWTObservables.get(control), BeanObservables.get(model, property), bindSpec); to: bindingBuilder.setTarget(SWTObservables.get(control).setModel(BeanObservables.get(model, property).setBindSpec(bindSpec).bind(); I find that easier to grasp when read. I would like to know what others think though because this is highly subjective. To form an unbiased opinion, I try to think about it in isolation of past conditioning to the older syntax.
Peter, I just reread Comment #47 in detail. When I initially read it, I missed some points. I am making up for that over here: "Pro: Allows new SWT widgets to be handled without changing public API. Con: Public API has no meaning. I envision a very focused class. SWTObservables would not be used to observe custom or third-party SWT widgets. External client code would have to provide the real logic anyway. Why ask SWTObservables to manage a process in which it has no meaningful contribution? Clients would be free to build a meta-factory around SWTObservables and their own custom factory logic if they want, but the databinding framework IMHO should not dictate the organization of that code. A change to the SWT type domain is an API change for clients, and I see no reason that this should be different for data binding. On the contrary, it makes perfect sense that the API should make new capabilities public and obvious. E.g. if SWT adds a new Calendar widget in 3.3, a human should be able to read the SWTObservables JavaDoc and immediately know if they can observe Calendars or not. Better yet, the JavaDoc can clearly describe which Calendar property will be observed." I envision a very focused class too. :-) Handling new widgets is an optional feature, which we do not have to have. However, I am not sure I understand the con. Maybe this is subjective. I would rather have developers rely on one class (SWTObservables) for all SWT observable needs, following the pattern of protected variations. In other words, if a new widget is used, we simply configure its observable factory, and poof, developers can now easily bind it just like binding any other widget, using SWTObservables.observe. Data-binding does not dictate the organization. Developers can always choose not to configure an observable factory as part of SWTObservables, and instead use an uber-factory that they create. Moreover, basic SWT widgets that are supported can always be documented in JavaDoc regardless of the appraoch. Nonetheless, having one syntax for binding widgets is great for ease-of-use and learnability in my opinion. If there are strong reasons to avoid this approach though, I would understand though I guess I need better understanding of them. "Pro: Factory logic is broken into smaller, more narrowly-defined pieces. Con: Dynamic typing instead of static typing. A major goal of this (evolving) proposal is to make the behavior of the framework easier to understand. One reason this is problematic now is because there is no type information on the parameters for API methods. This means that method dispatch is not fully determined until runtime, so it is impossible for either human or machine to trace an execution path statically. Any kind of dynamic indirection based on runtime type has this problem. Plus, casts are ugly and scary. I don't want to make it standard practice to require them. Generics are not available to us because of compatibility issues." In the past, the parameter type was Object, which indeed created problems. With the new approach, there is a static type: Control. "SWTObservables.observe(Control c)" This makes it clear that you can observe controls with the SWTObservables uber-factory, without having to write many observe(XXX) methods (or getXXX methods), which clutter the design of the class and make it less-focused. I personally do not see any need for casting (I do not like casting at all by the way and am a big fan of generics). The observe method (actually observeValue and observeList methods) will return an IObservable (actually IObservableValue and IObservableList) that can be used as is in the dbc.bind method (actually bindValue and bindList :-) ). "Con: Proliferation of classes Your design implies the creation of at least n+1 concrete classes and m interfaces, where n = number of SWT control classes and m = number of IObservable-derived interfaces. As of this writing, n=14 (including swt.custom) and m=5 => 20 new types. It would be reasonable (but inadvisable) to implement separate concrete classes for each valid combination of control and IObservable type, which would perhaps double the number of new API types and set an upper limit of n*m+m. Thats a lot of API surface area for clients to deal with. In contrast, I'm proposing 1 new type that would replace an existing type (SWTObservableFactory) for a net API growth of zero." As mentioned before, the "micro" observable factories will be hidden behind SWTObservables, which will be the only point of contact from the user stand-point. There will be only four basic interfaces IObservableValue, IObservableList, IObservableValueFactory, and IObservableListFactory. Also, a concrete ObservableValue or ObservableList per control that needs it, and a corresponding ObservableValueFactory or ObservableListFactory. I think the total number of units (classes/interfaces) can be calculated as follows: Total = SWTObservables + 4 basic interfaces + (observable and observable factory per widget) = 1 + 4 + (2 * n) where n is number of widgets = 5 + 2*n (not bad) We can potentially make each ObservableValue also an ObservableValueFactory, thus simplifying the total into 5 + n. I heard many times that it is a common OO design practice to delegate work to highly cohesive small classes instead of having one big class do all the work. To my knowledge, that provides better extensibility, easier testability, and simpler maintainability (I don't like to ever scroll through a big class file, or worse yet a big class unit test, to maintain it).
(In reply to comment #55) > "My main issue with this, other than I don't like 20 constructors in a class" > > No need for 20 constructors or inheritance when following the implementation at > the bottom of Comment #48. One constructor suffices and the API can be easily > extended (through addValueObservableFactory method) due to use of > delegation/strategy patterns. But if you just have one constructor we now a class that doesn't have a way to describe the support it provides. This was one of the reasons why we went from having the generic factories in the first place. > In any case, if we don't want the constructor approach, and prefer the factory > method approach instead, what is your opinion of the highly configurable > approach outlined in Comment #46? Please read #46 and #48 in detail if you > haven't done so to get a full understanding (note that #48 has some extra line > breaks). Idea contributions to improve the approach are greatly appreciated. > Remember though, the code was written for demonstration purposes and is not > complete or flawless, so it is better to focus on the big picture for now. I promise I'll reread those comments but I'm still confused as to what we're defining the problem to be. It started out (comment 38) being that the API wasn't intentional enough (which I'm interpretting as meaning it was not expressive enough). But that has morphed into the desire for an ultimately flexible factory API. These are two separate problems and I'd like to understand which one we're discussing.
re: comment 49 In the case of SWT observables, there is an important non-stylistic difference between constructors and factory methods. Consider the following: /** * Constructs an IObservableValue of type String that will observe * the text contained in the given Text box. */ public SWTObservableValue(Text text) { ... } Text t = new Text(parent, style); IObservableValue v1 = new SWTObservableValue(t); IObservableValue v2 = new SWTObservableValue(t); // Results in two observable values that track the same value. Each must // be disposed when done. class SWTObservableValue { /** * Returns an IObservableValue of type String that tracks the * text contained in the given text box. The observable will be disposed * automatically when the widget goes away. Must not be disposed * by callers. */ public static IObservableValue observe(Text t) {... } } Text t = new Text(parent, style); IObservableValue v1 = SWTObservableValue.observe(t); IObservableValue v2 = SWTObservableValue.observe(t); // Returns the SAME observable value twice. It does not need to be disposed. --- Static factory methods have the following benefits over constructors: 1. In the case of SWT observables, static factories can return a shared, managed instance. This simplifies client code and avoids redundant observables tracking the same property. By itself, this should justify using static factories - at least for the SWT observables. 2. Static factories avoid tying the code to using a particular implementation class. 3. In the case of constructors, you face a tradeoff between: a) Creating a lot of classes, one class per property being observed. b) Creating a lot of constructors in the same class. c) Creating a single loosely-typed constructor Each of these has a problem: a) is inefficient and verbose, b) is hard to read, and c) goes against the goal of having a simple, staticly typed contract on every observable created. 4. With static factories, we only need one API class for all SWT observables. Even with a single loosely-typed constructor, the constructor approach would require at least 3: one that implements IObservableSet, one that implements IObservableList, and one that implements IObservableValue. I agree with Peter's remarks in comment 47. My preference is for static factories... but I'd still be happy to see this addressed, even if we go with the constructor route.
(In reply to comment #54) > (In reply to comment #53) > > public SWTObservableValue(Control control, int style) {...} > > My main issue with this, other than I don't like 20 constructors in a class, is > if a consumer was to overload the constructor and created their own style > future updates to data binding could break their code. Styles only work if you > have full control over them. Plus this would sometimes be specifying a style > and others it would be specifying the attribute, like enabled, to observe. > Styles are difficult and they make somewhat ambiguous API. Static factory > methods allow you to have more explicit names that aren't tied to the class > name and I think that's a virtue of the current approach. It allows the > consumer to easily state that they want to bind to the enabled property or to > just create an observable for the text. SWT has done a good job of maintaining > styles but it's a difficult thing to do and for something as dynamic and wide > open as the attribute to bind to and customization of an observable I think it > would be a risky path to take, especially for a 1.0 release. OK; let me see if I'm understanding your concerns: 1) (parent, style) can't express an attribute to bind to. 2) Factory method names can be more expressive than just overloaded constructor arguments 3) Style bits aren't ideal. In general I mostly agree with these concerns. Let me clarify the proposal a bit and then I hope that most of them will go away. Regarding point #1, I agree that using (parent, style) as the specific constructor arguments will not work 100% of the time, or that we should try to use this convention when wrapping non-SWT classes. I'm mainly talking about the set of cases where you're binding to the default property of some SWT control. For example, (parent, style) simply doesn't make sense when binding to a JavaBean property. In that case, you'd use something like: new JavaBeanObservableValue(person, "firstName"); Regarding point #2, I feel that this is a matter of style. For example: public static IObservableValue getText(Text text) {...} and public SWTObservable(Text text) {...} convey exactly the same API information. But I prefer the latter because in the former, there is a redundancy between "getText" and the parameter type "Text". I also respect your preference not to read through lots of overloaded constructors. However, I have pretty strong feelings against using static methods if we can avoid them. Peter suggested way back in this discussion that we could have a real (instantiated) factory object rather than static factory methods. I would greatly prefer this option over using a proliferation of non-overridable static methods. But this would require always instantiating a factory or keeping a singleton around. From the API consumer's point of view, this is pretty complex. Which brings me full-circle back to my proposal: overload constructor arguments. There's no factory object to instantiate, nothing to configure, and we get the full OO benefits of being able to override our behaviors later. Regarding point #3, about style bits not being ideal, I guess I don't have really strong feelings. I've written a lot of SWT custom controls and never found style bits to be a big problem, even when I've overloaded and overridden them. But this is really a side point. I can sacrifice the style bits and still get at least 80% compatibility with tooling. For example, XSWT already knows how to work with somthing that claims to be an SWT control but doesn't have a style bit parameter. VE might too, but I haven't checked. > But to jump back a few comments I am a little confused about comment 38. It > said that the API needed to be more intentional. Was your discussion over the > old API (DBC and factories) or the current SWTObservables implementation? I'm > speaking of methods like... > > SWTObservables.getText(Text text, int event) > > To me this is intentional and there's no ambiguity as to what is being bound to > and what the outcome will be. I agree that this is intentional (reading the code correctly communicates the programmer's intentions). That's not what I view the discussion to be any more, though. My point in comment #53 and also above in this comment is that I believe static factory methods and overloaded constructors are equally intentional, if they both use concrete classes as arguments. Stefan disagreed on this point, so let me address his concerns here. They were: > 3. In the case of constructors, you face a tradeoff between: > a) Creating a lot of classes, one class per property being observed. > b) Creating a lot of constructors in the same class. > c) Creating a single loosely-typed constructor I'll take these in the order C, A, B: C) I agree that we don't want to do this. A) With the constructor approach, you will have a single wrapper class per property being observed, but there is no need for this to imply a single actual Observable. For example, given a private static Map previousObservables = new HashMap(); inside SWTObservableValue, you can write something like: public SWTObservableValue(Text text) { delegate = previousObservables.get(text); if (delegate == null) { delegate = new TextObservableValue(text); previousObservables.put(text, delegate); } } From an API consumer's point of view, this will yield identical semantics to a static factory method that returns the same IObservableValue given the same control. B) On the one hand, you have a proliferation of overridden constructors. On the other hand, you have a proliferation of static methods (may be overridden or not). Either way, it seems to me that we wind up with a proliferation of something, hence I still contend that it's really a question of style. :-) That was a long discussion (phew!), so let me summarize the concerns I have right now: a) Maximizing extensibility of the API. Static methods aren't overridable, and as a result aren't as extensible as constructors. b) Maximizing compatibility between our API and existing JFace and SWT APIs (we are a JFace project, after all. ;-) Thanks for everyone's thoughtful comments. I hope this clarifies where I'm coming from. I'm also generally supportive of Andy's suggestion using the constructor arguments, but I prefer to overload constructors with concrete classes so that the JavaDoc and content-assist immediately tell the API consumer what classes we can wrap. We just discussed this and I think he generally agrees on this point now.
(In reply to comment #60) I can't believe we have 60 comments already! > That was a long discussion (phew!), so let me summarize the concerns I have > right now: Great. I like reducing the number of open issues. :-) > a) Maximizing extensibility of the API. Static methods aren't overridable, > and as a result aren't as extensible as constructors. I am not sure I fully understand what you mean by extensible. It can't be API extensibility - the ability to grow the API over time, because that is easy: just add more factory methods. So I guess you mean extensibility by clients. One way to extend this is to define your own factory class and add methods there, but this does not seem to be what you mean either because you want overridable methods. Overridable factory methods are useful if you want to take code that was written against the original factory methods and extend or modify what happens when those factory methods are called, without having to modify the caller of the factory methods. Maybe you want to keep track of all created observables - but nothing prevents clients from defining and instantiating their own observable implementations, or you want to wrap all created observables in some way (same counterargument applies). What are the use cases you think of? Wouldn't they require a special contract with clients anyway, for example that they only create observables through one of your factories? Couldn't you achieve the same effect by introducing some kind of über-factory (in your code, not the data binding framework) that your clients are required to call? But maybe I am not on the right track with my analysis of what you might mean, because constructors are *not* overridable without having to change client code. I am open to changing the factories to have non-static methods. Admittedly, this makes it a little less convenient to use the API, but it would also remove the 'singleton' code smell that static factory methods have. When I started writing the current (new) factories, I briefly considered going this route, but decided to stick with static methods in the end. This enables clients who are lucky enough to be able to use Java 5 to statically import the factory methods. I say lucky because then they could use verbs instead of nouns which according to http://steve-yegge.blogspot.com/2006/03/execution-in-kingdom-of-nouns.html is a good thing. Constructors are not a good choice IMO, for all the reasons Peter, Brad, and Stefan have already listed. > b) Maximizing compatibility between our API and existing JFace and SWT APIs > (we are a JFace project, after all. ;-) My golden standard of API design is the Java collection framework. It uses concrete classes and constructors when clients have to make an implementation decision - LinkedList vs. ArrayList, or HashMap vs. TreeSet. In these cases, the concrete classes offer additional API that clients may want to call, so it makes sense to expose to clients the exact class implementing the behaviour. For example, there is LinkedList.removeFirst(), which is not available for all java.util.Lists. If there are no additional methods for clients to call, the implementation classes are not exposed. Instead, clients call static factory methods like Collections.unmodifiableList() and the like. I now have written more than I wanted to write. Sorry! Lets try to converge on this before we reach 100 comments. ;-)
(In reply to comment #61) > (In reply to comment #60) > > I can't believe we have 60 comments already! Hey; we're not even coming close to blowing out Bugzilla's limits yet. ;-) > > a) Maximizing extensibility of the API. Static methods aren't overridable, > > and as a result aren't as extensible as constructors. > <snip/> > I am open to changing the factories to have non-static methods. Admittedly, > this makes it a little less convenient to use the API, but it would also remove > the 'singleton' code smell that static factory methods have. I think that in the end this is a reasonable compromise. It eliminates all the wrapper factories in the constructor factory proposal. It supplies all the extensibility I want (ability for clients to override factories; ability to inject your own factory implementation into a unit test). I think if we're going to have singletons, let's just supply a SWTObservables.getDefault() that (possibly creates and) returns a default instance, but also supply a SWTObservables.setDefault() so that unit tests can inject their own implementation when necessary. Then we'll only have one singleton per factory and each will be in a single well-defined place. :-)
> This enables clients who are lucky enough to be able to use Java 5 to > statically import the factory methods. This is a really useful convenience. I'd hate to see us sacrifice it. Compare: // Obtained via instance methods IObservableSet selection = JFaceObservables.getDefault().getSelection(viewer); // Obtained via static methods (JFaceObservables is statically imported) IObservableSet selection = getSelection(viewer); > I am open to changing the factories to have non-static methods. > Admittedly, this makes it a little less convenient to use the API, but it > would also remove the 'singleton' code smell that static factory methods have. The trouble is, we can't make that factory client-implementable since we'll want to continually add methods to it... so such a factory won't really help with test frameworks even though it will cost us the convenience of static methods. There is definitely a need for generic factories - but not everyone is going to need the indirection all the time. Clients shouldn't be forced to use a less-convenient interface in situations where they don't need the extra flexibility. We can easily build a generic factory on top of static factories at any time with no cost to memory or performance.
(In reply to comment #63) > > This enables clients who are lucky enough to be able to use Java 5 to > > statically import the factory methods. > > This is a really useful convenience. I'd hate to see us sacrifice it. Compare: > > // Obtained via instance methods > > IObservableSet selection = JFaceObservables.getDefault().getSelection(viewer); > > // Obtained via static methods (JFaceObservables is statically imported) > > IObservableSet selection = getSelection(viewer); This doesn't have to be either-or. Simply make the static methods delegate to the default singleton. Then I can still override the behavior whenever I need to and we can provide folks with the convenience of static methods if that is what they want. Here is one use-case for this: Suppose I'm writing a unit test and need getSelection(viewer) to return an IObservableSet that is different from the standard one in some way. In other words, I need to inject my own implementation of IObservableSet into the test. With static methods alone, I don't see how I can do this; it appears that I'm stuck because there is no way to override the behavior of a static method in Java. However, if the static method delegates to JFaceObservables.getDefault().getSelection(viewer), problem solved, as long as there is also a JFaceObservables.setDefault() for changing the default implementation.
> Simply make the static methods delegate to the default singleton. Then I > can still override the behavior whenever I need to and we can provide folks > with the convenience of static methods if that is what they want. This sounds like a good compromise. As a user of the framework I'm perfectly happy with what you've just described. As one of the implementers of the framework I would still prefer a static-method-only implementation, as this results in less API and will be simpler to code. For this reason, I will poke at the JUnit use-case briefly to see if there is a way we could address it with static methods only. If not, no harm done... I'm happy with the API you've described. > Suppose I'm writing a unit test and need getSelection(viewer) to return an > IObservableSet that is different from the standard one in some way. I can think of two use-cases: Case 1: When writing a general-purpose utility that can act on any type of observable, the generic object is going to recieve the observable as a constructor arg or in a setter of some sort. When I instantiate it in my app I can bind it to a viewer selection. When I instantiate it in my JUnit test, I can bind it to something else. We won't need a layer of indirection within SwtObservables itself since it would be the JUnit test that selected the observable to bind to. For example, I could use this to test UnionSet, ControlUpdator, or other general-purpose code. Case 2: If I'm writing special-purpose code that explicitly requests the selection from a particular viewer and does something with it, I wouldn't want to change the observable being bound to in my JUnit tests. If the JUnit test were to inject an observable that was different from the one in my app, then I wouldn't actually be testing the same code I was deploying. For example, I might have a dialog box containing a TreeViewer that ties the enablement of the Okay button to the selection in the viewer. My JUnit test might change the selection in the viewer and assert that the button enablement updated correctly, but it would never bind the enablement of that button to anything other than the selection in that particular viewer since that's not what happens in the app. In case 1, we already have enough of an abstraction to swap in a different observable implementation during unit testing... and in case 2, we wouldn't want to swap in a different observable implementation during unit testing since doing so would invalidate the test. My question: Is there a use case where it would be both benefitial to change the observable implementation at the factory level and impossible to do so at a higher level.
(In reply to comment #65) > > Simply make the static methods delegate to the default singleton. Then I > > can still override the behavior whenever I need to and we can provide folks > > with the convenience of static methods if that is what they want. > > This sounds like a good compromise. As a user of the framework I'm perfectly > happy with what you've just described. To clarify what we are talking about: would the non-static methods be protected, so that clients would always call the static methods?
How does a singleton work in the multiple classloader environment of plugins? For example if you set the default factory for your view would that affect the factory used in other views?
(In reply to comment #66) > (In reply to comment #65) > > > Simply make the static methods delegate to the default singleton. Then I > > > can still override the behavior whenever I need to and we can provide folks > > > with the convenience of static methods if that is what they want. > > > > This sounds like a good compromise. As a user of the framework I'm perfectly > > happy with what you've just described. > > To clarify what we are talking about: would the non-static methods be > protected, so that clients would always call the static methods? I don't have a really strong preference. I guess, for starters, let's have the instance methods be protected and someone gripes, we can always make them public later. But we can't go the other direction.
> To clarify what we are talking about: would the non-static methods be > protected, so that clients would always call the static methods? > I don't have a really strong preference. No preference here either. > For example if you set the default factory for your view would that > affect the factory used in other views? Yes, that's what has been proposed. However, can someone please elaborate on where this would be used? I'm not suggesting it's a bad thing, but things will be simpler to code if we could do without it. In particular, why would one replace the factories in their JUnit test instead of just selecting different bindings?
(In reply to comment #69) > > To clarify what we are talking about: would the non-static methods be > > protected, so that clients would always call the static methods? > > > I don't have a really strong preference. > > No preference here either. > > > For example if you set the default factory for your view would that > > affect the factory used in other views? > > Yes, that's what has been proposed. Hmmmm; This would seem to be a strong argument for including an instance method implementation as well as the static one. We would also need an API contract that the default singleton is to be replaced only in unit tests. If a person wants to inject behavior in a plugin, one must do it in a non-static instance of the factory. What are use-cases for this: - Undo/redo - Converting between "" and null in SWT controls and POJOs respectively - Automatically strimming unwanted spaces from the start or end of strings - setEnabled(false) doesn't always redraw correctly when using FormToolkit; our temporary workaround adds a wrapper Observable around bindings to the enabled property and does control.getParent().redraw() after the property value is set. Why don't I want to just write an uber-factory? Because if I do that, I will lose all of the strong typing benefts of the new API. I could use a static Decorator pattern, but that requires re-coding all the methods in SWTObservables in my CustomSWTObservablesFactory so that they delegate to SWTObservables rather than just overriding the ones I need and inheriting the rest. So while I could do that, I'm pretty strongly against that. Conclusion: I'm pretty strongly in favor of keeping both the instance method API and the static API public and documenting that setDefault() is *only* for use in unit tests or standalone applications. > However, can someone please elaborate on where this would be used? I'm not > suggesting it's a bad thing, but things will be simpler to code if we could do > without it. In particular, why would one replace the factories in their JUnit > test instead of just selecting different bindings? Any time I need to instrument the Observables to record data being passed, count method invocations, etc.
(In reply to comment #70) > Conclusion: I'm pretty strongly in favor of keeping both the instance method > API and the static API public and documenting that setDefault() is *only* for > use in unit tests or standalone applications. What if someone misunderstands how it works and does the wrong thing? I would hate to debug that issue as it would only occur with a specific configuration, depend upon the order that the plugins are loaded, and lead to a very long debugging session. A factory like the one being spoken of above has always been more useful for me as an application level construct. By being at the application level it could be tailored to the use cases of the application and not of the library, which it is today.
(In reply to comment #71) > A factory like the one being spoken of above has always been more useful for me > as an application level construct. By being at the application level it could > be tailored to the use cases of the application and not of the library, which > it is today. Can I read this as: Brad prefers public instance methods, to be called on a factory object, over static methods to be called on the factory class?
(In reply to comment #72) > (In reply to comment #71) > > A factory like the one being spoken of above has always been more useful for me > > as an application level construct. By being at the application level it could > > be tailored to the use cases of the application and not of the library, which > > it is today. > > Can I read this as: Brad prefers public instance methods, to be called on a > factory object, over static methods to be called on the factory class? You can read this as Brad thinks the Databinding API shouldn't try to facilitate an overridable factory. I will want all the things that Dave and other have described but I want that in my own class. I think the API that databinding should provide shouldn't attempt to optimize itself for a specific use case if that makes it difficult on others. So for the issue of having a settable default I don't want someone else to be able to set this and have adverse effects on my code that is expecting the factory to behave in a specific way. To me that's a deal breaker. I like the current static methods as they don't prohibit me from creating my own API that uses this class. In the use cases that I just want an observable for a Text I have a simple and concise sytax in order to do this.
(In reply to comment #73) > (In reply to comment #72) > > (In reply to comment #71) > > > A factory like the one being spoken of above has always been more useful for me > > > as an application level construct. By being at the application level it could > > > be tailored to the use cases of the application and not of the library, which > > > it is today. > > > > Can I read this as: Brad prefers public instance methods, to be called on a > > factory object, over static methods to be called on the factory class? > > You can read this as Brad thinks the Databinding API shouldn't try to > facilitate an overridable factory. I will want all the things that Dave and > other have described but I want that in my own class. I think the API that > databinding should provide shouldn't attempt to optimize itself for a specific > use case if that makes it difficult on others. So for the issue of having a > settable default I don't want someone else to be able to set this and have > adverse effects on my code that is expecting the factory to behave in a > specific way. To me that's a deal breaker. I like the current static methods > as they don't prohibit me from creating my own API that uses this class. In > the use cases that I just want an observable for a Text I have a simple and > concise sytax in order to do this. I would agree except the ability to cleanly add undo/redo support throughout an application isn't exactly a corner-case scenario. And I don't see how we can easily support undo/redo with the static method solution. (Please prove me wrong. :-)
We had a call about this and decided to stick with static factory methods. The naming convention will be observe<Attribute>(SomeObject observed, ...), where <Attribute> is the name of the observed attribute or getter as in e.g.: SWTObservables.observeText(Text observed, int flags) SWTObservables.observeVisible(Control observed) SWTObservables.observeSelection(Spinner observed) SWTObservables.observeMax(Spinner observed) <insider-joke> Those who already smoked the weed of the Überfactory and can't give up are advised to roll their own factories, or move to stronger stuff like e.g. AOP. </insider-joke> I will go ahead and make these changes. (Unless a new discussion starts and we reach the 100 comment mark on this bug.)
Thanks everyone for following up on this, even when I fell off the earth. :-) I just checked on HEAD, and this looks really good to me. I will certainly be adding higher-level abstractions on top of SWTObservables, but this is what I want/expect from the framework.
Changes to SWTObservables released >20061112.
Verified that my changes are in I20061213-0800.
