Community
Participate
Working Groups
Suppose I want to restrict an Integer field to the range 1-10. I would use an IntegerValidator and a String2IntConverter on the target2model side. I would then write an IDomainValidator to restrict the integer value coming from the String2IntConverter to the range 1-10. But this would not result in ideal behavior. The partial validation in the IntegerValidator would permit any value from Integer.MIN to Interger.MAX. But since the domain model only allows a value in the range 1-10, it makes no sense to permit a leading - sign or more than two digits in the field. The way things stand currently, the user could enter 365 in the field and wouldn't find out until they tried to leave the field that this is an invalid value. The solution appears to be to change the partial validation algorithm to the following: Call the conversion validator's partial validation method. If this succeeds: Call the conversion validator's full validation method. If this succeeds: Use the converter to convert the value to the domain's data type Pass converted value to partial validation method on domain's validator. If we do this, then the domain's validator will be able to do both partial and full validation. If we do this, then IDomainValidator will have all the same methods as IValidator, so it can go away and we can just use IValidator in both instances.
This behavior seems to be a bit untypical. Let's consider how JSF does this (http://www-128.ibm.com/developerworks/java/library/j-jsf3/index.html) when processing request: 1. Perform conversion 2. Perform validation I.e. if text field is used to input a date value, it's first converted String-to-Date (or Calendar) and then validation is performed, for instance, to check that date is in the future (for a flight reservation). This makes sense, b/c domain (business) validation deals with domain objects and doesn't know how to convert text to a business object. It looks like the only difference b/w JSF and SWT/JFace is that we can additionally perform partial validation. But this is additional "enhancement" task and probably should be treated as such, by adding special type of validation. Additionally, partial and full validation work quite differently: if partial validation fails - control's value is not modified, conversly full validation doesn't affect control's value but only it's binding into the model. Another significant difference is that these two types of validations can be potentially triggered at the completely different times: partial validation is triggered by the input while full validation can be delayed (currently EXPLICIT policy). For these reasons, I believe that IDomainValidator should stay and IValidator should probably only have partial validation logic. In this case the process will still be: 1. Perform partial validation When ready to bind: 2. Perform conversion 3. Perform domain validation
(In reply to comment #1) > For these reasons, I believe that IDomainValidator should stay and IValidator > should probably only have partial validation logic. > In this case the process will still be: > 1. Perform partial validation > When ready to bind: > 2. Perform conversion > 3. Perform domain validation There are four kinds of validation in a rich app: 1) After a keystroke (partial validation on the conversion). 2) Validate that a conversion will be successful before attempting it (full validation on the conversion). 3) Partial validation on the domain value (after conversion). 4) Full validation on the domain value. If you have a Text bound to an int where the business logic dictates a range 1-50 is implemented by the following set of validation rules: 1) will permit empty string "", plus an optional "-" followed by any number of digits that add up to an absolute value of less than Integer.MAX_VALUE. 2) is the same as (1) except that it doesn't permit an empty string. This is because having an empty string temporarily during data entry is valid but you can't convert an empty string to an int value. 3) permit any positive integer made of two digits. This is because the user should be permitted to edit digits separately. Ie: it's more user-friendly to permit 59 to be entered accidentially, but then edited to 49 by letting the user edit the first digit independently of the second. This gives the user the maximum reasonable freedom to enter data the way that makes sense for them, while keeping the user from performing unnecessary extra work. 4) only permits integers 1-50. This is the actual business rule.
(In reply to comment #2) > 2) Validate that a conversion will be successful before attempting it (full > validation on the conversion). Is it just possible to try conversion and process possible exceptions? Or provide ConversionErrors in the conversion interface? Something like (similar to Spring Framework) Object convert(Object value, ConversionErrors errors) This could also take over some responsibilities for the step #2. > 2) is the same as (1) except that it doesn't permit an empty string. This is > because having an empty string temporarily during data entry is valid but you > can't convert an empty string to an int value. It's still possible though. What if Integer will be used? In this case null will be a valid value and it'll be the responsibility of the domain validator to perform the check. > 3) Partial validation on the domain value (after conversion). > 4) Full validation on the domain value. So it means you would still keep domain validation that will be working after the conversion? Will you keep domain validation steps, but change it to use IValidator interface? Otherwise, I can see how partial domain validation can improve the process overall, b/c full domain validation works only before the actual binding.
(In reply to comment #3) > (In reply to comment #2) > > 2) Validate that a conversion will be successful before attempting it (full > > validation on the conversion). > > Is it just possible to try conversion and process possible exceptions? Or > provide ConversionErrors in the conversion interface? Something like (similar > to Spring Framework) > Object convert(Object value, ConversionErrors errors) You could just process exceptions, but then you can't supply as nice an error message to the user. It's also possible to have converters return a ValidationError if they can't convert, but I think it's cleaner to keep validation and conversion concerns completely separate. > > 2) is the same as (1) except that it doesn't permit an empty string. This is > > because having an empty string temporarily during data entry is valid but you > > can't convert an empty string to an int value. > > It's still possible though. What if Integer will be used? In this case null > will be a valid value and it'll be the responsibility of the domain validator > to perform the check. Sure, you could do that. I was just giving one example of where the requirements for partial validation are different from the full validation requirements, which is why we have two methods on our IValidators to handle these two concerns. > > 3) Partial validation on the domain value (after conversion). > > 4) Full validation on the domain value. > > So it means you would still keep domain validation that will be working after > the conversion? Yes. > Will you keep domain validation steps, but change it to use > IValidator interface? Yes. > Otherwise, I can see how partial domain validation can improve the process > overall, b/c full domain validation works only before the actual binding. Yes.
a. It can sometimes make sense to allow more than 2 digits in a text field that ultimately accepts the range [1-50]. Example: The starting value is "10", and the user wants to end up at "15". 1. click on the zero character -> cursor is between the digits 2. type "5" -> text reads "150" 3. press SWT.DEL -> text reads "15" b. Imagine a hypothetical generic type system that enforces static correctness for data bindings. T == Target object type I == Input value type M == Model object type V == Domain value type The players in validation would be typed like this: Binding<T extends IObservableValue<I>, M extends IObservableValue<V>, I, V> IValidator<I> IConverter<I,V> IDomainValidator<V> c. In practice, IValidator<I> is often tightly coupled with IConverter<I,V>, e.g. String2IntegerValidator needs to know the radix of the String representation, the number representation for the current Locale, etc. IDomainValidator<V> is not coupled to IConverter<I,V>, but is often coupled to M. The BindSupportFactory API enforces this, mapping I+V => IValidator<I>, I+V => IConverter<I,V>, and M => IDomainValidator<V>. d. "Partial domain validation", as described by Dave, is tightly coupled with IConverter<I,V> and with IDomainValidator<V>. (The concept of "digits" only makes sense when <I extends String> and <V extends Number>, and you only know that "two" is the right number of digits because of IDomainValidator<V>.) I assert that a class implementing this feature must have access to the input value to work properly, and not just the domain value. Let's hypothesize a new home for Dave's desired feature: IPartialDomainValidator<I,V> { ValidationError isPartiallyValid(I input, V convertedInput); } e. If I and V are core Java types, the binding framework can provide reasonable default implementations of IValidator<I> and IConverter<I,V> and the necessary BindSupportFactory mappings. Clients must provide their own IDomainValidator<V>, IPartialDomainValidator<I,V>, and a custom BindSupportFactory to map M => IDomainValidator<V> and I+M => IPartialDomainValidator<I,V>. f. IDomainValidator<V> would be much more reusable than IPartialDomainValidator<I,V>. Furthermore, it would be difficult to implement the latter without breaking the architectural separation between target and model. g. If a framework client wants to provide "partial domain validation" for a specific Binding, they canprovide a custom IValidator<I> that is tightly coupled with both IConverter<I,V> and IDomainValidator<V>. This works right now, without needing to add IPartialValidator<I,V> at all. h. The proposed new feature does not seem to provide a significant improvement, but it would cause a major increase in complexity. Clients can add the functionality themselves, if needed, without changing the framework's current design. Conclusion: It's just not worth it. KISS!
I am now looking at this again, and am not sure if we reached consensus on this. Why is it not simpler to remove IDomainValidator in favour of using only IValidator all the time, whether on the target side or on the model side? IValidator (after addressing bug 118429) will look like this: public interface IValidator { public IStatus validate(Object value); public IStatus partialValidate(Object value); } I believe Dave suggested doing the following when receiving a "value changing" event (the event which allows us to veto a value change): Call the conversion validator's partial validation method. If it does not return an OK status, veto the change If it returns an OK status: Call the conversion validator's full validation method. If it does not return an OK status, do nothing, just return If it returns an OK status: Use the converter to convert the value to the domain's data type Pass converted value to partial validation method on domain's validator If this does not return an OK status, veto the change Is this too complicated?
I think that in comment 5 Peter has a good point. We could go round and round about whether the validation should attempt to only allow 2 digits to be entered but I don't think that's the issue. What I gleaned from Peter's comment is that this type of validation needs to be done with knowledge of the input value and not just the model value, and for this use case I tend to agree for simplicity sake. So instead of doing this in the domain validator a custom target validator could be written and validation would occur during the target validation phase. Technically speaking you could still assert during partial domain validation (Dave's proposal) that the model value is [0, 99] (at this point it would be an int) but to me that seems a little confusing. Are there other use cases that this type of partial domain validation would be required? Also Peter's advocating that the consumer just specify a target validator that performs this validation and that this could be done with the current API. The sticky thing about this is that we default the validator so the API would be a little cumbersome to setup this but it wouldn't bee too bad. Since we only have one use case at this point in time I'd vote for not enabling partial domain validation but leaving ourselves open to it in the future, as long as Peter's proposal satisfies Dave's use case which I think it does. What about dumping IDomainValidator for the following: public interface IValidator { public IStatus validate(Object value); } public interface IPartialValidator extends IValidator { public IStatus partialValidate(Object value); } This way BindSpec would only know of IValidator on both sides. Then depending upon the binding it could check for the validator being an instance of IPartialValidator as partial validation doesn't make sense in all bindings, nor for all validation phases. The binding would just need to specify the type of validation it performs and when. In the future we could add partial domain validation by adding support for IPartialValidator in ValueBinding in the domain validation phase if deemed necessary.
A couple of comments: (In reply to comment #7) > public interface IValidator { > public IStatus validate(Object value); > } > > public interface IPartialValidator extends IValidator { > public IStatus partialValidate(Object value); > } Notice that validate and partialValidate both have exactly the same signature. I think that the only interface we need is the IValidator one. I don't know how we're specifying these things now that BindSpec is gone, but I suggest that the conversion (partial, full) validators and domain (partial, full) validators can just be separate parameters to the bind method. So you just have four IValidator parameters. Regarding converters and validators needing to have knowledge of each other, sure, in some instances they could. There's no reason some implementation of IConverter and IValidator couldn't be implemented that way. But the simplest cases don't need to be coded this way; consequently I don't think the API needs to know about that.
(In reply to comment #8) > I think that the only interface we need is the IValidator one. The reason I recommended two interface, but having IPartialValidator extend IValidator, was to keep from making the BindSpec API more complicated. Plus there will never be a partial validator without a validator and they seem to be tightly coupled. Partial validation is only used in Text. Therefore I didn't want to make the setup of all other use cases more verbose because of this one use case. > I don't know how we're specifying these things now that BindSpec is gone, It's still there.
(In reply to comment #9) > (In reply to comment #8) > > > I think that the only interface we need is the IValidator one. > > The reason I recommended two interface, but having IPartialValidator extend > IValidator, was to keep from making the BindSpec API more complicated. Plus > there will never be a partial validator without a validator and they seem to be > tightly coupled. Partial validation is only used in Text. Therefore I didn't > want to make the setup of all other use cases more verbose because of this one > use case. Good points, but I still don't like the redundancy between the two interfaces. Might there be another way to solve the problem that avoids the redundancy and better expresses the programmer's intentions (what I understand you're wanting)? > > I don't know how we're specifying these things now that BindSpec is gone, > > It's still there. Oh, cool. Man, I've got to pull HEAD again one of these days. :)
(In reply to comment #10) > Good points, but I still don't like the redundancy between the two interfaces. > Might there be another way to solve the problem that avoids the redundancy and > better expresses the programmer's intentions (what I understand you're > wanting)? I'm not sure there's a better way. There are alternatives but I think they're worse. 1. If we were to make validate(...) accept a "phase" parameter this could be problematic to consumers if we added a new phase and their implementation was coded incorrectly (using an else statement). 2. If we were to force these into separate validator implementation we would then be changing the API of the rest of the framework to accommodate and I'd prefer to avoid this if possible. We'd then still have to have some indication as to when each validator was to be invoked. This could mean having another type for partial validation which gets us back to the two interfaces. I personally don't mind the redundancy between the two methods. It gives a straightforward indication of what each is and when it will be invoked. To me it feels unambiguous and unintrusive.
OK
I actually prefer just having one interface. Here is how it would look like: BindSpec bindSpec = new BindSpec(); bindSpec.setPartialTargetValidator(new IValidator() { public IStatus validate(Object value) { if (((String)value).length > 2) { return ValidationStatus.error("Please enter at most two digits"); } else { return Status.OK_STATUS; } } }); bindSpec.setTargetValidator(new String2IntegerValidator()); I find a method setPartialTargetValidator() easier to explain than a solution that requires clients to also implement another interface for which we would check using instanceof. As for Brad's second argument, we already decided to change the way validation works (see bug 118429), so we are making breaking API changes anyway.
Hi All, First, I wanted to say that it's very good that this topic is being revisited, as databinding is esentail for the quality RCP application development. Just a few points here: 1) I wanted to narrow down the interfaces and entities to what's necessary. They are conversion and domain validation. In case of Text controls, both of this will be performed one after another before the value is ready to be bound into the model. First convertion will be done on a String value, converting it to an object of a domain type (e.g. number or date) and then domain validator will be asked to check if the resulting object is ok. Everything else are add-ons, although very usefull. I believe, it would help if this would be clear from the interfaces design. Also, please notice that for some UI controls partial validation doesn't even make sense. 2) While it is probable that partial validation is performed on the actual domain types (like in case of numbers b/w 1 and 50), it's not always possible. For instance, if it is a date/time field or a phone number it wouldn't make sense to try to convert values prior to validation. Most likely, partial validation in this case will have to deal with Strings. So I believe, in general, partial validation should be framed on strings. Plus, it'd be probably annoying for users to be reminded that date is invalid while they're still entering it. And very likely the most help in this cases would be to ensure that uses can't enter "illegal" characters for a particular type (e.g. letters when only digits expected, etc). 3) RE: > public interface IPartialValidator extends IValidator { Bases on the first two points, I believe this could be inconvinient. B/c: a) both methods would very likely operate on different data types: validator on domain types and parital validator on strings. b) The most likely candidates for the common/standard implementations will be converters and partial validators. For instance, converter "String2DateConverter" and partial validator "ParialDateValidator". Domain validators on the other hand are very specific from task to task, i.e. a number should be in the range, date can't be in the past, etc. So in case when we'd try to have partial validators (arguably a more stable class) to derive from domain validator (more likely to change), it would reduce code reusability.
Changes released >20061211. I discussed this with Brad and was able to convince him to go with my proposal from comment 13.
I apologize in advance for commenting on a closed bug. This is an easy change, I hope.... Would it be possible for the validate method to provide a reference to the Binding or at least the BindSpec for which it is currently acting? This would allow a singleton IValidator instance to coordinate with the IConverter, get additional info from domain objects, etc. to handle odd cases. The workaround with the proposed API would be to configure references to these other objects with instance fields, which means extra memory usage to maintain the instances and the wiring. It would also put some burden on the IValidator to keep itself in sync with any changes to the BindSpec, etc. My proposal: public IStatus validate(Object value, Binding binding); This would allow most people to ignore the complex cases where both the target and domain values are needed, and also makes it easy to handle things like L10n number and date formats efficiently. It allows the framework to maintain one authoritative set of configuration for the various parts of a Binding, but provides access when it is useful and cheap. All of the API contortions get cleanly swept aside, but complex things are still possible without crazy redundant bookkeeping. The downsides I see: 1. It smells a little unpure from a religious OO point of view, since it emphasizes singleton command objects instead of independently configured instances. 2. It exposes parts of the framework that were previously inaccessible at this point in the process, so there will need to be clear documentation re: what can be safely modified and what will cause horrible breakage. A read-only guarantee (or fail-fast behavior) would be really handy in this case.
Good points, Peter. ++1
Peter, could you have a look at IBindingListener? Does it allow you to do what you want? The BindingEvent contains references to the target and model observables, and allows you to set the validation result at any point in the process of transferring a value change from one end to the other.
BindingEvent doesn't quite do it. I can't see a way to get access to the IConverter, which is the most likely thing that I would want. It also seems awkward -- I might as well not have an IValidator at all, if I'm going to put critical parts of the computation in the IBindingListener.
(In reply to comment #16) > public IStatus validate(Object value, Binding binding); The change you proposed doesn't allow you to access the converter either. Maybe the name IBindingListener is wrong - I always thought that the main reason why we introduced IBindingListener was to support advanced validation needs. (Dave, please correct me if I'm wrong.) And yes, you would not use an IValidator at all when you deal with validation in the binding listener.
You two are both raising good points. Perhaps IValidator should go away entirely in favor of the binding events? The binding listener was introduced for advanced validation, and also as a hook for O/R mappers to know when to persist their models.
Verified that my changes are in I20061213-0800.
