Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
AW: AW: [riena-dev] Comments regarding Validation

Hi Christian,

 

your changes look good.

 

+1 for removing the IValidationRule

 

However there is a small issue with the semantics of the ValueBindingSupport#removeValidationRule method:

 

ValueBindingSupport vbs;

vbs.addValidationRule(ruleX, false);

vbs.addValidationRule(ruleX, true);

vbs.removeValidationRule(ruleX); // ???

 

From my POV this should remove both rules. I've added a (currently failing) test case to ValueBindingSupportTest that assumes this.

 

Regards,

Elias.

 

 

Von: riena-dev-bounces@xxxxxxxxxxx [mailto:riena-dev-bounces@xxxxxxxxxxx] Im Auftrag von Christian Campo
Gesendet: Wednesday, July 09, 2008 07:16
An: Riena Developers list
Betreff: Re: AW: [riena-dev] Comments regarding Validation

 

Ok so I did a little bit of testing and made some changes. (ME the non-UI guy :-) )

 

I changed the method addValidationRule(IValidator) to addValidationRule(IValidator,boolean validateOnEdit) (actually deprecated the first and did a new method)

 

So now for every IEditableRidget you can specify when the rule should be checked (rather than let the rule decide). Still the rule decides whether a violation is a block or not.

 

So for example

txtDate.addValidationRule(new ValidIntermediateDate(DATE_PATTERN), true); // does a loose check for date ON_EDIT (thats what the true is for)

txtDate.addValidationRule(new ValidDate(DATE_PATTERN), false); does a strict check on date (different rule) but not on edit but when the focus is lost (false)

 

So maybe Elias you can look at what I did and tell me what you think.

 

If you like it (and I hope you do), I think we can get rid off IValidationRule and stick to IValidator.

 

I also changed AbstractValidDate our DateValidator to use a different implementation. In the previous version it used a GenericValidator.isDate and that reported a date of "25.12.200x" as valid. So now I use the SimpleDateFormat itself and check on the parsePosition and compare it to the length. If not everything was parsed then the field is invalid. That seems to work a lot better.

 

Any feedback welcome. I've checked the implementation in and modified the example.client to use the new methods. If everybody likes it, I can remove the deprecated stuff and remove IValidationRule tomorrow.

 

Also modified our swing version so that it compiles.

 

thanks

 

christian campo

 

 

Am 09.07.2008 um 11:13 schrieb Christian Campo:



comments below

Am 09.07.2008 um 10:48 schrieb Gayk Wanja:



 

Hi Elias,

> you were right; I've fixed this:

> - ValidEmail and ValidExpression changed to be non-blocking. This fixed some issues with the example.

Something I messed up, thanks for fixing.

> Note that the point discussing some limitations of the current validation APIs still apply. In particular we should think about:
> - if validation time is decided by the rule implementer or the rule user

I'd vote for rule user (see my past comments on why I think this is the way to go).

I am still also undecided here. (see below)



> - if validation response (block, dialog, message, xyz) is decided by the rule implementer or the rule user

I'd vote for rule user, keeps rules more versatile.

I am not sure if that is true though. How versatile can a DateValidator get. It will validate a date and nothing else. Same as EmailValidator. Even a Validator for regex only does just that. I am currently undecided on this. Putting it on the rule user must mean that it is sensible to have different values for different usecases.



> - how to react when invalid values are introduced programmatically via ridget.setText() / ridget.updateFromModel()

I'd vote for throwing an IllegalArgumentException, so the programmer can take action on this kind of problem, for example converting old data from the database to a new format. It also provides a stacktrace to fix the bug where it happens. I personally prefer not to have an issue ripple through the whole applicaton and see the bomb explode in some entirely other location, just because some other bug prevented this problem to be seen immediately in the ridget.
In the end ridgets can also be uneditable, so the user may see there's something wrong but not have any possibility to fix it manually, so we'd get bug reports like: "There's a questionmark in my date field and the application won't save any data, telling me it's broken", instead of a proper stacktrace.

+1 for the IllegalArgumentException

> - if we want to update validation status when rules are added removed

That would be nice, wouldn't it? Alas "nice" does not necessarily mean "implemented in half an hour"..
So I guess I wouldn't vote for anything here :-).

+1



I'd like to add the question

- do we really need "direct writing" in our ridgets?

 Updating the model could happen on enter, on focus transfer, on closing the form (which included closing the application), on navigation and on manual updates. I see that "direct writing" causes some problems, like either you must block temporarily incorrect input (which is frustrating to the user and may not work at all for regex-rules for example) or write to a buffer and only write the last valid buffer to the model on keypress (leaving values in the model, which are correct, but not the ones seen in the user interface). I guess not having something as "direct writing" would get rid of those problems. Is the benefit of direct writing worth the issues it raises?

I think that direct writing is not a nice to have feature but a requirements from our users. Direct writing means that correct data can be directly written to the model and through a property change listener you can (it what ever way) react to these changes i.e. suggesting completion, filter result sets. Some rules also work a lot better if you use direct writing, because the rule is only checked when data is about to be written to the model.

removing direct writing would at least currently also allow all sorts of letters in number fields for example.

for regex validated fields I think the solution is to simply not block typing even if the rule fails. Marking the field is sufficient.



Regards,
Wanja

_______________________________________________
riena-dev mailing list
riena-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/riena-dev

 

regards

christian

 

_______________________________________________
riena-dev mailing list
riena-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/riena-dev

 


Back to the top