Hi Wanja,
I think your email contains very valuable input.
Personally, I agree that the framework would be more flexible if
the validation rule did not make assumptions on WHEN to validate and WHAT to do
when the validation fails. A couple of points to discuss:
1. I'm not sure what exactly you mean with 'highlight'. Is that
an error or warning marker shown next to the widget? Is that some characters in
the text widget being highlighted? The first should be possible; the second is
currently not possible, because we do not have a way to know which characters
in the input are invalid (this is information that each rule would have to
provide).
2. I believe the original motivation for having different validation
times (on_edit vs on_update) was that some validations can be too expensive to
be performed on every keystroke. Nevertheless implementation would be simpler
if we only supported one validation time (for example on edit). Currently the rules
seem pretty straightforward in terms of "expensiveness" but I'm sure
there are more complex examples in the wild.
3. Assuming that we want different validation times: Maybe the
validation time should be passed as a constructor argument instead of being set
via inheritance (as currently). Having to subclass a rule to change validation
time seems a bit awkward. The constructor argument also forces the developer to
make a choice WHEN to validate.
4. Regarding the rule's constructor: one idea would be to use
the '|' operator to be more swt-like: new
MinLength(3, IRidget.Hints.TRIGGER_DIALOG |
IRidget.Hints.TRIGGER_ERROR_HIGHLIGHT); However I'm open to either
style and the '|' operator places some constraints on the values used for the
constants.
Food for thought.
Regards,
Elias.
Von:
riena-dev-bounces@xxxxxxxxxxx [mailto:riena-dev-bounces@xxxxxxxxxxx] Im
Auftrag von Gayk Wanja
Gesendet: Thursday, June 19, 2008 02:28
An: riena-dev@xxxxxxxxxxx
Betreff: re: [riena-dev] Comments regarding Validation
First of all, sorry if my mail breaks some
threading in your mail client,
I have just subscribed to this list and need to copy some of the texts from the
archive.
> From: "Volanakis, Elias":
> [case a+b]
> user types in widget
> -> on edit validation -> update ridget text
> -> focus lost / CR -> on update validation -> update model
>
> [case c]
> ridget.setText()
> -> on edit and on update validation
> if !ok -> throw runtime exception; widget, ridget, model unchanged
> if ok -> update widget and model; widget, ridget, model have same
> value
>
> [case d]
> model.setValue("xxx"); ridget.updateFromModel()
> -> on edit and on update validation
> if !ok -> throw runtime exception; widget and ridget unchanged
> if ok -> update widget and ridget; widget, ridget, model have same
value
I second that.
If the application developer sets a value, he should be aware of what he sets,
so it's okay to raise an exception. He could check the rules on his own before
setting the new value, if he really needed to.
This is also how partial editing could be realized, when the rules were strict.
1. write user input to buffer
2. validate buffer
3. if buffer valid write buffer to model
I do fancy rules which are strict.
In my opinion rules have to check whether input is valid or not with nothing in
between. Whether or not or even when invalid input is accepted, should be up to
the class using the rule, not the rule itself.
If we had a textfield which wants at least 3 letters to be valid,
popping up a dialog and blocking the input on errors would me a user's
nightmare: as soon as he liked to correct the last character, he needed to
write the new character and delete the previous one, instead of dleting the
last one and entering the new one. Of course, everything that was acceptable
here would be highlighting the error, not showing a dialog.
Anyway, the application developer may still like to see a dialog when input
fails on CR/lost focus (where I usually think a dialog is the wort possible way
to indicate errors, as it interrupts the workflow of the user forcing him to
click a some button somewhere on the screen before he can go on and fix the
wrong input, but that's an entirely different issue).
So it should be possible for the application developer to configure his ridget
the way, that it (optinally) highlights while typing and (optionally) shows a
dialog on CR/focus lost. This is however ridget logic, not rule logic.
Because of that, I also dislike the idea that the rules return a fixed value
which tells when the rules likes to be evaluated, lke they do today (see
validationTime).
I would prefer if each rule, by default, was always evaluated (=on input) and
the developer could set the validation time to another value if necessary.
Maybe some more generic parameters to the rule would be a nice idea.
Imagine this (using some pseudocode here):
-- user code:
ridget.addRule(new MinLength(3, IRidget.Hints.TRIGGER_DIALOG,
IRidget.Hints.TRIGGER_ERROR_HIGHLIGHT));
ridget.addRule(new MaxLength(12, IRidget.Hints.TRIGGER_WARNING_HIGHLIGHT));
-- framwork code:
class AbstractValidationRule implements IValidationRule{
AbstractValidationRule(Object... hints){..}
//...
}
class MinLength{
private final int len;
MinLength(int len, Object... hints){
super(hintes);
this.len=len;
}
//...
}
Where the ridget does something like:
//... on input:
if(updateHighlightsOnInput){
for(IValidationRule rule : rules){
jointStatus.join(rule.validate(input)); //also joins hints
}
if(!jointStatus.isOk()){
if(jointStatus.containsHint(IRidget.Hints.TRIGGER_ERROR_HIGHLIGHT)){
setErrorMarker(jointStatus);
}else
if(jointStatus.containsHint(IRidget.Hints.TRIGGER_WARNING_HIGHLIGHT)){
setWarningMarker(jointStatus);
removeErrorMarker();
updateModel(...);//update anyway, this is just a warning.
}
}else {//everything is okkay
removeErrorAndWarningMarkers();
updateModel(...);
}
}
//... on update:
for(IValidationRule rule : rules){
jointStatus.join(rule.validate(input)); //also joins hints
}
if(!jointStatus.isOk()){
if( jointStatus.containsHint(IRidget.Hints.TRIGGER_DIALOG)){
triggerDialog(jointStatus);
}
if(jointStatus.containsHint(IRidget.Hints.TRIGGER_ERROR_HIGHLIGHT)){
setErrorMarker(jointStatus);
}else if(jointStatus.containsHint(IRidget.Hints.TRIGGER_WARNING_HIGHLIGHT)){
setWarningMarker(jointStatus);
removeErrorMarker();
updateModel(...);//update anyway, this is just a warning.
}
}else{ //everything is okkay
removeErrorAndWarningMarkers();
updateModel(...);
}
//...
This way the user could configure the ridget to
1. whether or not update any highlight on input (just on update)
2. show a dialog for severe errors and a highlight for minor ones
3. even highlight warnings, which do not block writing to the model, like
"texts of this length may be unusable with older
clients" or
"it is not recommended to use an abolute path, try
'$PROJECT/my/subdirectory/'".
What do you think?
Regards,
-Wanja-