Community
Participate
Working Groups
Copied from bug 75625: > > Even subclassing UpdateStrategy is not as simple as it > > should be because of the use of private methods (for example, getConverterMap() > > should be protected so a subclass can add its own converters). > > You are confusing the private implementation (converterMap) with the API > (createConverter). I don't see why a subclass cannot use its own implementation > to look up converters, delegating to the super implementation if it cannot find > anything. In fact, if I understand Ed correctly, EMF already has generic > capabilities for conversion and validation which could not adequately be > handled by putting specific converters into a map that is keyed by specific > value types. As I'm sure you know, inheritance and sublcassing is not the only mechanism available to implement extension. I was suggesting that UpdateStrategy could directly support extension by allowing other code to register converters with it. That would, IIUC, alleviate the need to also subclass DataBindingContext because it would not be necessarily to subclass UpdateStrategy. For example, this is the approach that Jakarta BeanUtils library does, allow custom converters to be registered instead of requiring subclassing to use them. See http://commons.apache.org/beanutils/commons-beanutils-1.7.0/docs/api/org/apache/commons/beanutils/package-summary.html#conversion.defining To me, this is a case where that kind of extension would be a little cleaner and more convenient for EMF's purposes. It would allow client developers to use just 1 specialized class (EMFObservables) instead of 3 (EMFObservables, EMFUpdateStrategy, and EMFDataBindingContext).
Just to make sure we're on the same page this is asking to be able to set converters staticly, yes?
I don't know... I am waiting for Eric's suggestions.
> I was suggesting that UpdateStrategy could > directly support extension by allowing other code to register converters with > it. Sounds like a static method to me. I don't like static methods like this because registering converters in such a way would affect all clients of the data binding framework, globally, without them being aware of it. Perhaps Eric wanted to have an instance method for this on DataBindingContext?
(In reply to comment #3) > Sounds like a static method to me. I don't like static methods like this > because registering converters in such a way would affect all clients of the > data binding framework, globally, without them being aware of it. That's my position as well, just wanted to make sure we're talking the same language. I think that this confuses the ideas of a low level framework/library, which we are, with an application framework, which we are not. Plus I'd rather not have someone else step on my converter configuration as such issues are difficult to reproduce.
Created attachment 80312 [details] A first draft implementation This is first draft, it's not documented, ... but the idea should be clear. I'm going to polish things if you think I'm heading in the right direction.
I'm still waiting on Eric to clarify the original issue.
Just to get some initial feedback to Tom... I think attachment 80312 [details] was what Boris was referring to in comment 3 but in my mind I'm questioning it's utility. The reason being that the customization of converters will be something that will be determined at the Binding level and not the DBC. The organization of Bindings in DBCs is performed for other reasons (e.g. they are in the same Composite and should be disposed when that Composite is disposed or the aggregation of Bindings to display an aggregate status). My guess is that consumers would have to then create more DBCs than necessary which would propagate into a new issue of how to manage DBCs. That's just off the top of my head. One idea that's been floating around my mind for a while is to have a Binding builder. The code to create Bindings is too verbose for me. Also builders allow for the setting of context without affecting others negatively. ValueBindingBuilder builder = ValueBindingBuilder.builder().withDataBindingContext(dbc); builder.bindModel(model1).toTarget(target1).build(); You could then set the instances that need to change for a new group of Bindings... builder.withTargetToModelConverterProvider(provider); builder.withDataBindingContext(dbc2); builder.bindModel(model2).toTarget(target2).build(); builder.bindModel(model3).toTarget(target3).build(); That's just the idea that's been floating around my mind. I haven't had the time to attempt to implement it yet. But for me it seems like a way to provide the maintenance of state in consumer code and not internal to the the core concepts of the library. It also makes the creation of Bindings less verbose and without having nulls passed to the methods which decreases the readibility of code.
In my mind the problem is that Databinding (UpdateStrategy/UpdateValueStrategy) is expecting that it provides a converter solution to all problems and it doesn't allow easy customization of it when the default-converts make no sense. Take for example my GWT thingy, I'll have to write my own NumberToString, DateToString, ... converts and it doesn't make sense that all those default-converters are loaded (if they could) and because the Databinding-Context only allows to pass UpdateValueStrategy, I'm sitting here now with: - GWTDatabindingContext (because I need to pass my own GWTUpdateValueStrategy) - GWTUpdateStrategy (Default-Converters make no sense so I replace them with my own implementations) - GWTUpdateValueStrategy, GWTUpdateListStrategy Whereas when the whole system could be configured from the out-side I would have write with GWTConverterProvider and all I'm ready. Taking your idea further do you think the Builder could take the IConverterProvider instead of the DatabindingContext? (we also have to do the same for Validators by the way)
(In reply to comment #8) > In my mind the problem is that Databinding (UpdateStrategy/UpdateValueStrategy) > is expecting that it provides a converter solution to all problems and it > doesn't allow easy customization of it when the default-converts make no sense. I'd like to refine that a bit. I think it's consumers who expect it to solve all problems although it wasn't designed that way, and it shouldn't. It's an abstraction that is meant to allow the consumer to customize the binding. It just so happens to provide default converters and validators. The piece that we're missing is the converter provider or a way to customize the defaults. Consumers seem to want a stateful object that can be used to generate strategies or at least that's the direction I think we're going with this. They're confusing the static map in our default implementation with this stateful object. Never say never but I doubt we'd ever allow anyone to add to this map. > Take for example my GWT thingy, I'll have to write my own NumberToString, > DateToString, ... converts and it doesn't make sense that all those > default-converters are loaded (if they could) and because the > Databinding-Context only allows to pass UpdateValueStrategy, I'm sitting here > now with: > - GWTDatabindingContext (because I need to pass my own GWTUpdateValueStrategy) > - GWTUpdateStrategy (Default-Converters make no sense so I replace them with my > own implementations) > - GWTUpdateValueStrategy, GWTUpdateListStrategy I agree that we need to expand things and ease the pain a bit. But I also think that you're use case is a little different than Eric's. You want to change just about everything that is the default. I think Eric just wants to tweak the default. Maybe we can solve both. > Whereas when the whole system could be configured from the out-side I would > have write with GWTConverterProvider and all I'm ready. You'd have to do a little more though, correct? You still need the GWTDataBindingContext, or some factory to create your DBC instance to provide your provider by default. If not everywhere you'd create a GWTDBC you'd need to pass the provider which would get old pretty quick. Let me know if I'm missing something there. > Taking your idea further do you think the Builder could take the > IConverterProvider instead of the DatabindingContext? (we also have to do the > same for Validators by the way) It could take anything and hopefully both. The point of the builder is to optimize to the pattern of the consumer creating bindings. I think the provider thought is on track. What I'm disagreeing with is attaching it to the DBC thinking that this will solve the problem. I think it would solve your problem but it won't solve the average consumer's problem who just wants to add a new converter every once in a while. In my mind the need is to be able to put instances in context to allow for the speedy construction of bindings. That's what the builder provides. It's a configuration that can be reused and cloned. Also I think that there could be a refinement to my previous code. Rather than... builder.bindModel(model3).toTarget(target3).build(); It could be... Binding binding = builder.bind(model, target); That way the model and target are not made part of the context but everything else is (e.g. DBC, converter provider, etc.). For your use case you could then just provide a factory that returns builders setup for your use case. It would come configured with your converter provider and you could even create a method to return one with a new DBC. Basically, I think that convenience is necessary but would like it to be separated from the core APIs allowing the two to flex separately.
(In reply to comment #9) > (In reply to comment #8) [...] > > > Take for example my GWT thingy, I'll have to write my own NumberToString, > > DateToString, ... converts and it doesn't make sense that all those > > default-converters are loaded (if they could) and because the > > Databinding-Context only allows to pass UpdateValueStrategy, I'm sitting here > > now with: > > - GWTDatabindingContext (because I need to pass my own GWTUpdateValueStrategy) > > - GWTUpdateStrategy (Default-Converters make no sense so I replace them with my > > own implementations) > > - GWTUpdateValueStrategy, GWTUpdateListStrategy > > I agree that we need to expand things and ease the pain a bit. But I also > think that you're use case is a little different than Eric's. You want to > change just about everything that is the default. I think Eric just wants to > tweak the default. Maybe we can solve both. > Would it make sense to design the system like a chain so the a converter can delegate request to the a more generic one if it doesn't know the answer to a request? > > Whereas when the whole system could be configured from the out-side I would > > have write with GWTConverterProvider and all I'm ready. > > You'd have to do a little more though, correct? You still need the > GWTDataBindingContext, or some factory to create your DBC instance to provide > your provider by default. If not everywhere you'd create a GWTDBC you'd need > to pass the provider which would get old pretty quick. Let me know if I'm > missing something there. Well I thought here in terms of extension points I could provide who inject the correct implementation for a given runtime environment (running under RCP I inject the default-implementation or better said nothing, when running under GWT I'm inject the custom replacement, ...). As you outlined this is not the task of Databinding-Framework but the API should make it at least possible :-) In fact I already have an abstraction-level above Databinding who manages this so setting up a bound text control looks like this: -----------8<----------- AbstractBoundWidget control = BoundTextFieldControl.createText( composite, SWT.BORDER, ingeniumPackage.Literals.CLIENT__GIVENNAME ); form.registerWidget("givenname", control); control = BoundTextFieldControl.createDate( composite, SWT.BORDER, ingeniumPackage.Literals.CLIENT__BIRTHDAY, GlobalPreferences.getDateFormats() ); form.registerWidget(PATIENT_BIRTHDAY, control); // .... form.bindAllWidgets(ctx, clientObservable); -----------8<----------- In my case it even creates the Text-Control which is different between GWT and RCP. If you read this carefully you also see that I'm not in need of a IConverterProvider in Databinding-Context and if I want to i don't even need any DefaultConverters provided by the Databinding-Lib because I know exactly which converters to assign to UpdateValueStrategy#setConverter(). For me the most important thing is that UpdateStrategy and UpdateValueStrategy are freed from the converter implementations (Do you agree that they should not be part of it?). I think we both agree the IConverterProvider idea is the right one and should replace the implementations in UpdateStrategy and UpdateValueStrategy, right? On the other hand shouldn't leave this completely to the frameworks build above databinding which means we clean the UpdateStrategy and UpdateValueStrategy from the defaults and for backwards they are provided from an external class but define that in future Frameworks should provide them by setting meaningful converters using UpdateValueStrategy#setConverter()?
(In reply to comment #10) > (In reply to comment #9) > Would it make sense to design the system like a chain so the a converter can > delegate request to the a more generic one if it doesn't know the answer to a > request? I'd be concerned about performance of a chain. Pre 1.0 we had factories like this where we asked each implementation until we found one that provided the implementation. It was quite slow. I'd prefer a way to inspect an instance to determine what converters it provided up front rather than doing this when the converter is requested. > Well I thought here in terms of extension points I could provide who inject the > correct implementation for a given runtime environment (running under RCP I > inject the default-implementation or better said nothing, when running under > GWT I'm inject the custom replacement, ...). As you outlined this is not the > task of Databinding-Framework but the API should make it at least possible :-) I disagree. Extension points would be like exposing the static map. We don't want to allow someone to configure someone else's Bindings. In my mind this is up to an instance and to configure an instance you do so via a factory or some other instance level means. > For me the most important thing is that UpdateStrategy and UpdateValueStrategy > are freed from the converter implementations (Do you agree that they should not > be part of it?). I agree that we'd benefit from an abstraction that allows the consumer to provide a library of converters. > I think we both agree the IConverterProvider idea is the right one and should > replace the implementations in UpdateStrategy and UpdateValueStrategy, right? Delegation seems to be an appropriate approach. > On the other hand shouldn't leave this completely to the frameworks build above > databinding which means we clean the UpdateStrategy and UpdateValueStrategy > from the defaults and for backwards they are provided from an external class > but define that in future Frameworks should provide them by setting meaningful > converters using UpdateValueStrategy#setConverter()? We have to have a default for backwards compatibility but also for the fact that without defaults it would be impossible for the library to do anything out of the box. Your use case is as drastic of an edge case that you can get (you're JDK isn't even close to the normal assumptions that are made when writing java code). I think it's important that we agree on that. Your use case needs to be possible. But I think we need to make sure that Eric's is simplified above and beyond what we have today. It would be great if Eric could chime in so that we could get a better feel for what he's needing.
Tom, do you think you could come up with a way to solve your use case with internal API? Without knowing what Eric's use cases are I'm hesitant to want to commit to API for your use case since it's not something that most consumers will need. My guess is if the API was internal we could get it in pretty quick.
I apologize for my silence on this so far - I've been busy with client work and then had a few days off with no online access. Anyway, my use case is simply to override or augment the default conversions. Doing so today requires far too much code. An example I've already run into is EMF; because EMF observable values don't return the actual Class to getType() (that's another debate in another Bugzilla), none of the default Data Binding converters work out-of-the-box. Overriding just to make them work requires subclassing UpdateStrategy and DataBindingContext which I think it beyond what the average app developer should have to get involved in. You could argue that this is the responsibility of EMFObservables and its brethren, but the point is that overriding conversion is too much work. Another, simpler, example is the fact that ObjectToStringConverter automatically converts null to empty String. Again, overriding that behavior required me to write my own UpdateStrategy and createConverter() which is not a trivial thing to do in a robust way. Not difficult for framework developers but more than I think most app developers want to have to deal with, IMO. I definitely agree with Tom that conversion should be separated from UpdateStrategy and made easier to override. I like the chain design idea, and although I trust the DB team's past experience I would like to see some measurement that a chain would be practically slower - design should not succumb to optimization until performance is measured, IMHO. I have not had a chance to look at Tom's patch above, so I'm gathering what I can about it from the follow-up discussion. One last comment: I don't necessarily agree that allowing modification of the default map of converters in UpdateStrategy is necessarily "A Bad Thing." As I said, BeanUtils has used that approach to great success. I guess I don't see a scenario where "someone" mysteriously alters the default conversions "without me knowing about it." An app would have to initiate whatever code would alter the converters map, so it would "know" what is going on. Yes, there is potential for misbehaving code to make "bad" changes, but I'd rather see an Enabling attitude than a Directing one (http://martinfowler.com/bliki/SoftwareDevelopmentAttitude.html). Anyway, I'm happy to see a more modularized design where the notions of conversion, validation, update-triggering, etc are all cleanly separated and easily modified independently. Sorry this is so long; I didn't have time to make it brief.
(In reply to comment #13) > I apologize for my silence on this so far - I've been busy with client work [...] > One last comment: I don't necessarily agree that allowing modification of the > default map of converters in UpdateStrategy is necessarily "A Bad Thing." As I > said, BeanUtils has used that approach to great success. I guess I don't see a > scenario where "someone" mysteriously alters the default conversions "without > me knowing about it." An app would have to initiate whatever code would alter I disagree here, you I guess you are thinking in terms of RCP applications where you fully control all the plugins but think of eclipse-plugins who are going to use the databinding-framework in future. It would fail miserably if one plugin interferes with another ones notion of default-converters.
Marking for investigation in M4. I like the idea of a builder - it looks like a good way to reduce the boilerplate code. I agree with Tom that we cannot let clients add entries to our internal (static) map. Tom - non-API changes are fine with me, but you would have to drive that (i.e. provide a patch) if you want it in M3.
Removing 3.4 M3 target.
+1 on externalizing the default converter map (and code) from UpdateStrategy to an object like IConverterProvider, which would be passed to UpdateStrategy in the constructor. I'm not excited by the idea of Builders and think this is a separate issue. +0. I would hope these are kept as an optional utility and not made mandatory. I've been thinking how DataBindingContext fits into this topic. Perhaps we can pass an (optional) custom ConverterProvider into DataBindingContext as a constructor argument and have it used for all *default* bindings. UpdateStrategy objects constructed directly for passing to bind() would need the ConverterProvider as a constructor argument. This fits nicely with the "Null is default". It is also backwards-compatible, as the default shipped ConverterProvider is used if no custom ConverterProvider is given. This makes ConverterProvider the application-wide singleton and extension point for controlling binding type support which I feel is wanted in DataBinding. It would also allow extensions such as org.eclipse.core.databinding.extended/java5 to provide "standard" enhanced ConverterProviders to consumers.
(In reply to comment #17) > +1 on externalizing the default converter map (and code) from UpdateStrategy to > an object like IConverterProvider, which would be passed to UpdateStrategy in > the constructor. This makes sense to me. Separation of concerns is good.
(In reply to comment #17) > This makes ConverterProvider the application-wide singleton and extension point > for controlling binding type support which I feel is wanted in DataBinding. Maybe I'm not understanding this statement but you could use a ConverterProvider as an application-wide singleton (for your own code) but the library would not include the concept of an application-wide singleton. Correct?
I think we're in different time zones so it might be better for me just respond to everything at once... (In reply to comment #17) > +1 on externalizing the default converter map (and code) from UpdateStrategy to > an object like IConverterProvider, which would be passed to UpdateStrategy in > the constructor. I have no problem with this from a technical standpoint but I'm having a hard time seeing how it solves the issue that has been defined. The proposals so far have wanted an application wide conversion mechanism. A consumer of the library, as it stands today, can customize the converters via extension. But in every case it would force the code that is invoking dbc.bind(...) to know of the implementation that provides the customization of converters (whether it be the DBC itself or strategies). There's been an aversion to this in this bug. Even if you were to pass this on construction of a DBC the code would still have to know of these implementations. To me that's not a big deal but it seems like it is Eric (Eric, correct me if I'm wrong here). > I'm not excited by the idea of Builders and think this is a separate issue. +0. > I would hope these are kept as an optional utility and not made mandatory. It would be optional but it is still related in my mind but I'll get to that shortly. > I've been thinking how DataBindingContext fits into this topic. > ... I believe this is along the lines of what Tom proposed in comment 5. My response is in comment 7. Do you think that this customization goes hand in hand with the life cycle of a DBC? What happens if you're passed a DBC? Are you going to assume that the DBC contains the conversion routines that you need or do you associate it with the role of managing the life cycle of the Bindings created? My opinion so far has been that customization of converters is either done when the binding is created or in some other object. This gets me back to the builder. The builder that I'm envisioning doesn't tie itself to the life cycle of any existing JDB concept. It only allows you to reuse configurations, not just of converters, that you employ when necessary. For example you could have a builder that you employ when creating widgets in a dialog and you want to defer the update of your model until the user selects 'OK'. Since it is your own builder it would allow for you to provide conversion routines as well. The API of the builder would allow for simple construction of bindings but how a builder instance is chosen would be up to the consumer. My desire for wanting to expose a builder from the library would be to allow for consumers to still depend upon JDB API for most of what is written to create a binding. The only code referencing their own application specific implementations would be the code that is choosing the builder. This would keep the code as clean as possible and allow for flexibility at the builder level. Again, I don't disagree with a IConverterProvider but I'm questioning it's applicability here. But so far I am disagreeing with its association with a DBC instance. I'm worried that consumers would then start trying to use an application DBC rather than using DBCs for managing the life cycle of Bindings. I know that passing it to a DBC wouldn't be the only way to customize the conversion process but it has seemed so far that developers don't want to pass anything to a bind method other than the observables when it comes to conversion. Today they can already customize the conversion routines with an extension of the appropriate strategy but that doesn't seem like a popular idea.
(In reply to comment #20) > I think we're in different time zones so it might be better for me just respond > to everything at once... Yep.. UK, EST +5. I will do the same. (In reply to comment #7 - Brad's Response to comment #5 (draft impl) ) > My guess is that consumers would have to then create more DBCs than > necessary which would propagate into a new issue of how to manage DBCs. I don't think this will affect DBC lifecycle. I use hundreds of DBCs in my application [framework], in whatever scope is appropriate. I've checked and DBC placement would remain the same once ConverterProvider is introduced. If the DBC does not hold this app-wide default ConverterProvider for all bind() operations for me, I have to create UpdateValueStrategy() objects simply to express the same ConverterProvider every time. Granted I already do create my own UpdateValueStrategy objects in many cases (to customize the UpdatePolicy at the very least). This leads me to consider: what if I do without the proposed DBC convenience? There would then not be a single case left where I could use bindValue(model, target, null, null). This would be a shame. Defaulting to the most common use case is a key purpose of the DBC in my opinion (along with grouping bindings for disposal and group validation state as you mentioned). If anything it should provide even more convenience. For example my biggest use of custom UpdateValueStrategy pairs is to have modelToTarget POLICY_UPDATE and targetToModel POLICY_NEVER, i.e. a read-only binding. This could be made more convenient.. dbc.bindReadOnlyValue(), for example. Read-only state is at the binding level. Also in my application the use of POLICY_UPDATE vs. POLICY_CONVERT (i.e. immediate update or explicit save) is universally dbc-wide. I would like to specify this as a boolean default in the DBC. If a DBC is not to aid in defaulting, maybe it should not allow null to be passed for the update strategy args at all... In summary of the above (before I'm any closer to the edge of the topic), I think DBC should be as aware of every aspect of it's default binding strategy as possible. This means less junk to construct and pass into every single bind() call. Having thought about this I'm starting to latch onto your (reusable) builder idea. This would serve my purpose well. I'm now considering - based on my experience of developing with JDB: What is the separation of concerns between builder and DBC? I'm not sure we need two separate objects in this space. > I'm worried that consumers would then start trying to use an > application DBC rather than using DBCs for managing the life cycle of Bindings. I have explored this throughout my many uses of JDB, and it does not concern me. I have found that every DBC (some of very small scope, some wider), even in my complex use cases, shares a lot of common update strategy, but not all. I see this as *part* of the lifecycle of the group of bindings. The creation of bindings. The DBC is IMHO as responsible for this shared aspect of creation and defaulting as it is for disposal. > conversion. Today they can already customize the conversion routines with an > extension of the appropriate strategy but that doesn't seem like a popular > idea. Tried it.. It's too hard and seems ridiculous when you get into it. Much quicker to insert e.g. 2 bespoke lines into UpdateStrategy, which essentially is what I think we consumers are trying to achieve. Sorry this is so long. Been productive for me though :)
(In reply to comment #21) > If the DBC does not hold this app-wide default ConverterProvider for all bind() > operations for me, I have to create UpdateValueStrategy() objects simply to > express the same ConverterProvider every time. Granted I already do create my > own UpdateValueStrategy objects in many cases (to customize the UpdatePolicy at > the very least). This leads me to consider: what if I do without the proposed > DBC convenience? There would then not be a single case left where I could use > bindValue(model, target, null, null). This would be a shame. Defaulting to the > most common use case is a key purpose of the DBC in my opinion (along with > grouping bindings for disposal and group validation state as you mentioned). +1! > In summary of the above (before I'm any closer to the edge of the topic), I > think DBC should be as aware of every aspect of it's default binding strategy > as possible. This means less junk to construct and pass into every single > bind() call. > > Having thought about this I'm starting to latch onto your (reusable) builder > idea. This would serve my purpose well. > > I'm now considering - based on my experience of developing with JDB: What is > the separation of concerns between builder and DBC? > > I'm not sure we need two separate objects in this space. Perhaps we could make DBC's constructor accept an IBindingBuilder instead of an IConverterProvider. This would neatly encapsulate the creation of bindings aspect into BindingBuilder, leave lifecycle management of bindings as the only unique responsibility of DBC. > I have found that every DBC (some of very small scope, some wider), even in my > complex use cases, shares a lot of common update strategy, but not all. > I see this as *part* of the lifecycle of the group of bindings. The creation of > bindings. The DBC is IMHO as responsible for this shared aspect of creation and > defaulting as it is for disposal. Delegating DBC's binding creation responsibility to IBindingBuilder would facilitate this: you set the default target-to-model and model-to-target binding policy once on the builder, and then create your bindings with the DBC as usual. The bindings inherit behavior from the builder.
(In reply to comment #21) > (In reply to comment #7 - Brad's Response to comment #5 (draft impl) ) > > My guess is that consumers would have to then create more DBCs than > > necessary which would propagate into a new issue of how to manage DBCs. > > I don't think this will affect DBC lifecycle. I use hundreds of DBCs in my > application [framework], in whatever scope is appropriate. I've checked and DBC > placement would remain the same once ConverterProvider is introduced. What about DBCs that your code does not know the origin of? For example, what if I created a View and Eclipse passed me a DBC. What would you do then? To me this is the use case that we're not accounting for which will have an affect on the design of components. This would then require us to add more API to handle this and I'd like to avoid that if possible. > If the DBC does not hold this app-wide default ConverterProvider for all bind() > operations for me, I have to create UpdateValueStrategy() objects simply to > express the same ConverterProvider every time. This is what is the most interesting to me. Why do you do this? Why not just extend DataBindingContext and handle the defaulting of converters on your own? Again, I'm not against an IConverterProvider but you already have the ability to handle this use case without new API. So rather than... DataBindingContext dbc = new DataBindingContext(IConverterProvider provider); you will have... DataBindingContext dbc = new MyDataBindingContext(); or even... DataBindingConext dbc = DataBindingContexts.withDefaultConverters(); > Granted I already do create my > own UpdateValueStrategy objects in many cases (to customize the UpdatePolicy at > the very least). This leads me to consider: what if I do without the proposed > DBC convenience? There would then not be a single case left where I could use > bindValue(model, target, null, null). That wouldn't be true if you created an extension of DBC. > This could be made more > convenient.. dbc.bindReadOnlyValue(), for example. Read-only state is at the > binding level. Also in my application the use of POLICY_UPDATE vs. > POLICY_CONVERT (i.e. immediate update or explicit save) is universally > dbc-wide. I would like to specify this as a boolean default in the DBC. That's where the builder comes in. The builder could contain this state and could also potentially contain the bindReadOnlyValue(...) method. A design principle that I hold dear is that core objects should expose the API that *they* need to function and convenience should be elsewhere. The reason being that with most additions of convenience you lose a little bit of flexibility down the road when it comes to additions to the API. It's hard to know what you will need in the future so I tend to be pretty conservative in this regard. Creating optimizations for specific use cases can get hairy especially on something as core as DBC. If we were to add the method bindReadOnlyValue(...) it would mean that every DBC would have to honor this and know what "read only" meant. I'm not sure I'm willing to make that assumption. As an aside, a good example of where I think this separation of configuration versus API that the core object needs is with GridData and GridDataFactory. > If a DBC is not to aid in defaulting, maybe it should not allow null to be > passed for the update strategy args at all... Regardless, if that would be ideal or not it's not a possibility as it would be a nonpassive change. > I'm now considering - based on my experience of developing with JDB: What is > the separation of concerns between builder and DBC? DBC is the core API necessary to create Bindings. A builder is used for convenience in regards to storing state across the creation of Bindings and would hopefully provide a "fluent interface" for the building of Bindings. With it's current state it would then delegate the creation of the Binding to the DBC in context. This would hopefully make the code to create Bindings more terse and expressive as you would not be passing null to any method of the builder to convey that it should default something. Of course this is all a theory at this point but that would be the goal. > > I'm worried that consumers would then start trying to use an > > application DBC rather than using DBCs for managing the life cycle of Bindings. > > I have explored this throughout my many uses of JDB, and it does not concern > me. I think we're probably the same in our approaches here but not everyone is. We've had a few complaints about wanting to modify the converters application wide and I'd be concerned this would lead some down the wrong path. If we could provide an alternative that isn't attached to a DBC instance I lean in its direction if I feel it's appropriate and doesn't have trade offs, or trade offs that I'm not willing to live with. > I have found that every DBC (some of very small scope, some wider), even in my > complex use cases, shares a lot of common update strategy, but not all. > I see this as *part* of the lifecycle of the group of bindings. The creation of > bindings. The DBC is IMHO as responsible for this shared aspect of creation and > defaulting as it is for disposal. I don't really see that, or at least don't want that going forward. Defaulting is context dependent and is hard to do in a core implementation. If we were to employ a builder it makes the defaulting context sensitive as the consumer could chose a specific configuration for creating the Binding which doesn't depend upon a specific DBC instance. > > conversion. Today they can already customize the conversion routines with an > > extension of the appropriate strategy but that doesn't seem like a popular > > idea. > Tried it.. It's too hard and seems ridiculous when you get into it. Much > quicker to insert e.g. 2 bespoke lines into UpdateStrategy, which essentially > is what I think we consumers are trying to achieve. Maybe I'm missing something here in your response but we have to live within some boundaries. Writing 2 lines into UpdateStrategy would never be possible. My guess is that you're talking about the defaulting of these strategies. If you were to create your own strategies in a DBC extension I don't see how this would be ridiculous. I think we'd be able to clear up a lot of the questions if we had some prototype code. A couple of other bugs are a higher priority to me at the moment. But maybe I'll get an itch and prototype something soon. I think that would help us to at least see something concrete and be able to not speculate as much.
(In reply to comment #22) > Perhaps we could make DBC's constructor accept an IBindingBuilder instead of an > IConverterProvider. This would neatly encapsulate the creation of bindings > aspect into BindingBuilder, leave lifecycle management of bindings as the only > unique responsibility of DBC. I'm having a hard time seeing why that would be advantageous. In my mind the builder will just delegate to the DBC in context to create the Binding. It would default the values that are in context. Having the builder know of the DBC but not the other way around would allow for a good separation IMO. The builder will contain the API for convenience, not the DBC. Basically, I don't want to see dbc.bindValue(target, model, null, null) in my code. Also I'm still hung up on the idea of wanting to tie default configuration to an instance of DBC.
(In reply to comment #23) > (In reply to comment #21) > > Granted I already do create my > > own UpdateValueStrategy objects in many cases (to customize the UpdatePolicy at > > the very least). This leads me to consider: what if I do without the proposed > > DBC convenience? There would then not be a single case left where I could use > > bindValue(model, target, null, null). > > That wouldn't be true if you created an extension of DBC. Subclassing doesn't work in all situations. Indeed the point of this bug is to not have to subclass to override the defaults. As noted in comment #0 the need to subclass core classes for every new category of types and converters can quickly get out of hand. Sooner or later you'll need to bind two APIs that aren't supported by the default converters. Each will have their own DBC subclass, and the diamond shaped inheritance problem will rear its ugly head. Making this concept a pluggable strategy sidesteps the whole problem, *and* makes it easy to override the defaults in the simpler situations. > A design principle that I hold dear is that core objects should expose the > API that *they* need to function and convenience should be elsewhere. Agreed. I also hold dear to "favor composition over inheritance." :)
(In reply to comment #24) > (In reply to comment #22) > > Perhaps we could make DBC's constructor accept an IBindingBuilder instead of an > > IConverterProvider. > > I'm having a hard time seeing why that would be advantageous. In my mind the > builder will just delegate to the DBC in context to create the Binding. It > would default the values that are in context. Having the builder know of the > DBC but not the other way around would allow for a good separation IMO. The > builder will contain the API for convenience, not the DBC. Basically, I don't > want to see dbc.bindValue(target, model, null, null) in my code. Also I'm > still hung up on the idea of wanting to tie default configuration to an > instance of DBC. Unfortunately, default configuration is already tied to DBC. It's unfortunate that the converter provider approach wasn't explored pre-1.0. In the interest of keeping the API consistent, extracting those defaults out of DBC with a strategy pattern seemed the best approach. BindingBuilder was merely the first thing that came to mind. One concern I have with the DBC(IConverterProvider) constructor is there might be (now or in the future) other default behavior that people want to override. For example, what if we wanted to start providing default pre-convert validators, e.g. for string-to-number conversions? Should the constructor argument be an object that encapsulates all these default behaviors?
Surely want the added value and convenience we are discussing to go into JDB API, not my private codebase in subclasses of JDB core. This is my aversion to subclassing, I know it is possible to achieve what we want by subclassing or modifying JDB, but instead we should put this useful and generally useful stuff into JDB for all consumers to use. I am still determined as I have said before to help JDB become more featured, convenient to use and rich with type support out-of-the-box. We can shorten the time taken to adopt JDB in a project. Consumers should certainly be writing domain-specific converters and validators, when they find they need to bind those types. For this they subclass. Also new kinds of observable, same with these. I use custom observables extensively, such as to an extended form of commons beanutils DynaBeans. This is all domain-specific work and valuable to the consumer, and not suitable for inclusion in JDB core. New consumers should not have to subclass or extend the framework itself to improve it in areas such as binding convenience and ease of registering domain-specific type support. These areas need improvement so lets improve them. I don't see any problems with backwards compatibility, either. Lets just design it right :)
If the DBC is not involved in common binding spec, builders will have to be chainable. Either this or it still must be possible to add a second layer or binding specification per binding, at the builder.bind() command. I have both 'common' binding spec throughout a context, e.g. explicit save (modelToTarget POLICY_CONVERT), as well as per-binding constraints such as "required field" and other "field constraint" validators. It would only be possible to put the former into a builder for re-use, the rest is configured individually per binding. Secondly if we do use a builder object for the shared defaults, can we avoid having to create both builder objects and DBCs at the same point? I foresee this happening. I agree that code would be useful at this point.
(In reply to comment #23) > That's where the builder comes in. The builder could contain this state and > could also potentially contain the bindReadOnlyValue(...) method. A design > principle that I hold dear is that core objects should expose the API that > *they* need to function and convenience should be elsewhere. The reason being > that with most additions of convenience you lose a little bit of flexibility > down the road when it comes to additions to the API. It's hard to know what > you will need in the future.... Some context here for those who've been around awhile that I hope will be helpful: In pre-1.0 days we had the uber-factory. We eliminated it to simplify the API and because we discovered through use that it reduced the extent to which the code clearly expressed the developer's intentions. Brad's basically proposing a way to bring back the uber-factory, but without its drawbacks: the core API stays simple; the convenience-API stays extensible (as long as the builder itself is designed extensibly), the code you write becomes very intentional... Not sure about the literate programming approach. I'm not fond of unit testing frameworks that use this, but I've got an open mind about it. At first glance, I like this idea very much.
I was starting to get a bit cross-eyed on this issue so I reread the entire thread. I concur with Brad that DBC having an IConverterProvider reference seems wrong. The only reason it needs a reference is to pass it to UpdateValueStrategy constructors, which is only necessary if you pass a null UpdateValueStrategy to dbc.bindValue. Using a binding factory eliminates the need for this field since it can create its own update strategies directly and not rely on DBC to create them. Following the conversation between Brad and Matt Carter about subclassing DBC to modify default behavior, I think that's fine for framework writers but is asking asking a bit much for regular JDB consumers. We shouldn't forget about the after-get validators either. UpdateValueStrategy has code that looks up default validators based on what default converter was used (if any). Custom converters should probably be coupled with an appropriate validator through an IValidatorProvider interface. It seems the consensus (correct me if I'm wrong) among JDB committers is that: * Now that the API is released the default converters will stay the way they are * Going forward the method for overriding converters is to pass an IConverterProvider either to the UpdateStrategy constructor or to the binding builder. If I've misunderstood or misrepresented what anyone said, please correct me.
(In reply to comment #30) > I concur with Brad that DBC having an IConverterProvider reference seems wrong. > The only reason it needs a reference is to pass it to UpdateValueStrategy > constructors, which is only necessary if you pass a null UpdateValueStrategy to > dbc.bindValue. Using a binding factory eliminates the need for this field > since it can create its own update strategies directly and not rely on DBC to > create them. I agree as well. In hindsight, it was not a very good idea to add the convenience methods bindValue() and bindList() to DataBindingContext, with all the defaulting that we have. I prefer the name binding builder over binding factory because it does more than just create bindings, it also configures them. > Following the conversation between Brad and Matt Carter about subclassing DBC > to modify default behavior, I think that's fine for framework writers but is > asking asking a bit much for regular JDB consumers. Yes, especially because you quickly run into wanting diamond-shaped inheritance structures to combine the (say) EMF defaults with the JFace defaults. > We shouldn't forget about the after-get validators either. UpdateValueStrategy > has code that looks up default validators based on what default converter was > used (if any). Custom converters should probably be coupled with an > appropriate validator through an IValidatorProvider interface. From what I remember, the two aspects are related to the point that it might be easier to have one provider interface for both converters and validators. > It seems the consensus (correct me if I'm wrong) among JDB committers is that: > * Now that the API is released the default converters will stay the way they > are > * Going forward the method for overriding converters is to pass an > IConverterProvider either to the UpdateStrategy constructor or to the binding > builder. +1. We should start writing code. To program is to understand... It would be nice to use this discussion to increase the number of snippets we have. So far, we don't have enough snippets that show how data binding can be used in common scenarios that involve validation, conversion, read-only bindings etc.
Created attachment 82601 [details] Builder Prototype <insert prototype disclaimers here> The key classes to look at are ValueBindingBuilder, IValueBindingBuilderDelegate and IStrategyDefaultsProvider. All of the implementations are prototype quality. I copied the converter and validator defaulting into CoreStrategyDefaultsProvider so this is duplicated at the moment. No tests have been written and the only code invoking this is the hello world snippet. If I can find some time to write up some more examples I will but I'm not sure when that will be. ValueBindingBuilder - the core builder class that manages state for reuse of configuration when creating value bindings. IValueBindingBuilderDelegate - the externalization of creating the strategy classes and then setting the converters and validators on these classes. The reason for externalizing this is so that if someone has their own extensions of the strategy, for example to configure doSet(...), we need to expose the ability for them to create their strategy instance. The reason for the prepare(...) method is so that converter and validators can be defaulted for all validation phases and not just after get. IStrategyDefaultsProvider - the abstraction for providing converters and validators.
+1 (In particular I now agree that DBC should not be involved in StrategyDefaultsProvider, or any defaulting beyond existing API).
Tom, can you point me to the EMF databinding code in CVS? I'm investigating the binding builders prototype and need complete picture of the problem..
http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.emf/org.eclipse.emf/plugins/org.eclipse.emf.databinding/?root=Modeling_Project
Mass update - removing 3.4 target. This was one of the bugs I marked for investigation (and potential fixing) in 3.4 but I ran out of time. Please ping on the bug if fixing it would be really important for 3.4, and does not require API changes or feature work.
(In reply to comment #36) > Mass update - removing 3.4 target. This was one of the bugs I marked for > investigation (and potential fixing) in 3.4 but I ran out of time. Please ping > on the bug if fixing it would be really important for 3.4, and does not require > API changes or feature work. > Well, this discussion has evolved into a new API and feature, so I guess your offer to ping is kind of hollow ;-) Still, I think a group of well-informed people on this discussion have all agreed that the way it works now is very unwieldy and there is a path forward (if you can see it through the mass of words here). I consider it tragic if there was not something done on this for 3.4 - we would essentially be putting the reliable use of whatever comes out at least 2 more years - 1 more year until we'd get it in 3.5 and then another year of having to support 3.3/3.4 where the new features would not be present. That would be a tragedy from my perspective as a plugin and RCP developer who relies heavily on DB. Could we not make a provisional API out of the patches here?
We are well past the API freeze for 3.4 unfortunately, and as of today, also past the feature freeze. Working on the builder stuff could be bootstrapped in the examples project though.
For interesting background, see bug 234003 - it contains a draft eclipse.org article which talks about "internal DSLs" - creating domain-specific "languages" that make use of method chaining and other techniques in order to end up with "good looking" Java code.
Seems tragic that this idea withered; I accept my part of the blame since I, too, forgot about it. Now we've missed the 3.5 boat as well. The last tangible activity seems to have been Brad's patch; did any committer ever review that or any consumers ever give it a whirl?
(In reply to comment #40) > Seems tragic that this idea withered; I accept my part of the blame since I, > too, forgot about it. Now we've missed the 3.5 boat as well. > The last tangible activity seems to have been Brad's patch; did any committer > ever review that or any consumers ever give it a whirl? The problem is that this takes somebody who drives it forward. Matthew has been very busy with the new properties API, but I am sure he has not forgotten about the builder API ideas. Also, Ovidio seems to be thinking about this again, see bug 183055 comment 8.
I've reviewed it but have refrained from comment thus far. In my personal work with data binding I only do plain old beans-to-widgets bindings without a ton of specialized validation / conversion logic. So while I see the value in this proposal I don't have enough practical need for it personally to know whether the design is right. Maybe this should be released early in 3.6 as provisional API to give people a chance to use it and provide feedback.
I'm coming back to DataBinding after a few years away. Did this code or any similar Builder concept ever make it in?
(In reply to comment #43) > I'm coming back to DataBinding after a few years away. Did this code or any > similar Builder concept ever make it in? No, it doesn't look like it. PW
This ticket was part of an old effort that is no longer actively pursued. If a similar effort is made in the future it is better to create new tickets, so I'll close this one.
