AW: [riena-dev] Comments regarding Validation

["Drossel, Carsten" <Carsten.Drossel@xxxxxxxxxxxx>]

Hi Elias,

> > case c: !ok -> update widget and ridget. Model not updated because of
> > the failed validation.
> > case d: !ok -> update widget and ridget to display the incorrect
> > value already in the model.
> 
> I thought of the behavior quoted above too, but there is a slight
> difficulty with it: Some validation rules fail with
> ERROR_BLOCK_WITH_FLASH and assume the invalid input is blocked while
> the error marker is flashed briefly. Now an invalid text is forced into
> the widget, via ridget.setText("iaminvalid"). The rule fails but the
> text is not blocked. The marker will flash and disappear while the
> widget still contains invalid text. As far as I can tell this cannot be
> fixed without changing the current APIs (i.e.
> IValidationCallback.validationRulesChecked(status)), but I am open for
> ideas.

You are right, there are some problems with the suggested behavior. When a normal textfield is used the solution could be not to "ERROR_BLOCK_WITH_FLASH" but to "ERROR_ALLOW_WITH_MESSAGE" i.e. to paste the invalid text into the field and add a permanent error marker. But when using a special date textfield the String has to match the date pattern. "99.99.9999" could be pasted and marked as error but something like "April 21th 1963" just will not fit. What happens in the Swing application now is what happens when the user tries to paste something like that into the datefield: the text remains unchanged and the error marked flashes. But since setText(String) is triggered by the application code the error marker flashes for no apparent reason from the users point of view.

So I guess throwing a runtime exception is the best solution. If illegal values from within the application are possible the application must check them before trying to paste them into a textfield. Then the user could be notified about problem in some other way e.g. by a dialog "Illegal data in the database. Please contact your database administrator. [ok]".

Greeting,
Carsten


-----Ursprüngliche Nachricht-----
Von: riena-dev-bounces@xxxxxxxxxxx [mailto:riena-dev-bounces@xxxxxxxxxxx] Im Auftrag von Volanakis, Elias
Gesendet: Mittwoch, 18. Juni 2008 02:51
An: Riena Developers list
Betreff: AW: [riena-dev] Comments regarding Validation

Hallo Carsten!

Thanks for the useful info.

1. With regard to re-validation on adding: Since we need this, I suggest rechecking on add / remove, because add / remove will not be used that frequently anyway. I don't think we need an extra method (i.e. revalidate()) and it eliminates one more thing the developer would have to rember (i.e. r.setText(); r.addRule(); r.revalidate()). It seems better without that extra method call.

2. Regardind invalid values:

> case c: !ok -> update widget and ridget. Model not updated because of 
> the failed validation.
> case d: !ok -> update widget and ridget to display the incorrect 
> value already in the model.

I thought of the behavior quoted above too, but there is a slight difficulty with it: Some validation rules fail with ERROR_BLOCK_WITH_FLASH and assume the invalid input is blocked while the error marker is flashed briefly. Now an invalid text is forced into the widget, via ridget.setText("iaminvalid"). The rule fails but the text is not blocked. The marker will flash and disappear while the widget still contains invalid text. As far as I can tell this cannot be fixed without changing the current APIs (i.e. IValidationCallback.validationRulesChecked(status)), but I am open for ideas. 

Currently my implementation throws an exception when doing ridget.setText() / ridget.updateFromModel() with an invalid value. An alternative would be keep to the old input if the new input is invalid. 
 
Kind regards,
Elias.

-----Ursprüngliche Nachricht-----
Von: riena-dev-bounces@xxxxxxxxxxx [mailto:riena-dev-bounces@xxxxxxxxxxx] Im Auftrag von Drossel, Carsten
Gesendet: Tuesday, June 17, 2008 08:12
An: Riena Developers list
Betreff: AW: [riena-dev] Comments regarding Validation

Hi Elias!

First referring to 3.:
when we migrated the validation from the preceeding closed-source Spirit framework, we tried to blend it with the existing validation in Eclipse (org.eclipse.core.databinding.validation) and to use as much of the Eclipse code as possible. One problem that we could not solve with the existing Validator however was partial checking. It should be possible to check rules already while the user is typing. E.g. with the date pattern "dd.MM.yyyy" the input "31.02." should be immediately marked as an error because it cannot be completed to a valid date. While the input "28.02." does also not match the date pattern there should be no error marking while the user is typing. But when he moves the focus to the next field "28.02." must be marked as an error because the year is missing. So a date validator should have different results depending on whether the user is still typing or not.

The solution we came up with was to split the validator in two (as you suggested as solution b) for bug 236990). There is a ValidIntermediateDate validator that checks whether a String is the prefix of a valid date and a ValidDate validator which checks that a String is a valid date. We had to extend the interface IValidator to specify the time when a validator is applied. We added a new interface IValidationRule with a method getValidationTime(). It returns ON_UPDATE_TO_MODEL when invoked on a ValidDate and ON_UI_CONTROL_EDITED for a ValidIntermediateDate.

Obviously more rules must be split like that. So far is has been done only for date but as you observed zip-code and email address are other examples.

Or maybe there is some cool way we didn't think of yet to solve these problems without adding another interface like IValidationRule...? :-)

Referring to 1. and 2.:
The focus so far was to make a control robust against a fixed set of end-user errors. So far there was no use case to change the rules after initialization. I think this could be an issue though. It is not likely that a date field suddenly becomes a zip code field. But the rule for a zip code field may change depending on the selection of a country combobox. So adding or removing a ValidationRule should indeed trigger another check!

2. is about some faulty input set by the application code. Could there be a use case for that? Maybe if the application is only displaying what is already in a database. Anyway I've tried cases c and d in the Swing application and I got rather inconsistent results e.g. depending on whether I was using a normal text field or a date field. In case c with a date field the invalid date is displayed in the text field which is marked as error. When using a normal text field with the rule ValidEmailAddress the text is not replaced and the error marker flashes shortly.

I think the best behaviour would be:
case c: !ok -> update widget and ridget. Model not updated because of the failed validation.
case d: !ok -> update widget and ridget to display the incorrect value already in the model.

Greetings,
Carsten


-----Ursprüngliche Nachricht-----
Von: riena-dev-bounces@xxxxxxxxxxx [mailto:riena-dev-bounces@xxxxxxxxxxx] Im Auftrag von Volanakis, Elias
Gesendet: Dienstag, 17. Juni 2008 08:55
An: Riena Developers list
Betreff: [riena-dev] Comments regarding Validation

I just finished adding validation to the SWT text ridget. Below are a
couple of comments with regard to the validation APIs (IEditableRidget)
and the text field.

1. Currently no re-validation is performed when adding / removing
validation rules. Consider this:

ridget.addValidationRule(numbersOnly)
ridget.setText("abc"); // invalid

but

ridget.setText("abc");
ridget.addValidationRule(numbersOnly); 
// valid until the user changes the text or the next setText()

We should consider if this is acceptable. At the minimum it should be
documented. If we want to change it we have to think about re-validating
with every add / remove rule or if there should be some kind of
revalidate() method.


2. Currently validation is mostly concerned with the text that is
entered by the user. However in reality there are 4 ways the text can
change:

a. keyboard entry 
b. ((Text)ridget.getUIControl()).setText() // equivalent to a; will not
be discussed further
c. ridget.setText("foo")
d. ridget.bindToModel(myBean); myBean.setValue("foo");
ridget.updateFromModel();

According to my understanding of the existing code cases c. and d. are
not handled. Christian and I discussed this on the phone. My current the
SWT implementation follows the behavior described below. I welcome your
thoughs (esp. if you are familiar with the swing implementation).

[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


3. Some existing validation rules seem to be too strict because they
block partial values that are incorrect. However while inputting
character-by-character we start with a partial value that is not correct
(example email) but will be correct after the input is finished. 

Example #1: see bug 236990.
Example #2: new ValidExpression(GERMAN_ZIP); This requires five digits.
Editing 12345 to 12346 is ok. Entering 12345 character by character is
currently not possible.

How was this handled in the past?

I welcome your insights.

Kind regards,
Elias.

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