PDA

View Full Version : [FIXED] Poor validation performace in form panel



21 Dec 2012, 9:26 AM
Prelude:
As we know, getErrors method from Ext.from form.field.Field realize validate on field.

Question:
Does anyone know, how many times this method will be perform after open window with form containing (for example) 30 columns ?

Assuming:
Data entered into the form by loadRecord are correct.

Answer:
it will be:
total_count_calling_of_getErrors = Cartesian product value number of fields + min 2 * number of fields


Where
1. min2 :

initialize()-> hasInvalidField() from [Ext.form.Basic]
loadRecord() -> setValues() loop on each field [form]
2. Cartesian product results from the series of events


For 30 columns we get a round number of 960 times!


Don't believe, just try: http://jsfiddle.net/AndrzejSulej/LrRsu/

Summary:
I understand that we need to be sure of the correctness of the data, but that's an exaggeration ...:))


Private opinion:

The problem is in the validation cycle events (simple graph after calling loadRecord and next setValues):
[field]checkChange() -> onChange() -> validate() -> isValid() {getError() -> fireevent ('validitychange)} ->
[form] checkValidity() ->hasInvalidField() (loop for fields ) ->
[field] isValid() {getError()}

loadRecord and setValues should suspendCheckChange (similarly as it's done in the method initValue (Ext.form.field.Field)) and on the end of loadRecord we should validate form. Reason: sometimes it happens that we want to do cross-validate, so we have to see all values in fields.Some developers use parallel validation of the model also.
I don't understand why getErrors performs validation. In my opinion it should return actually (last) error collection. I know it's a little bit revolution in code involves a lot of changes.
According to my philosophy:
validate() - should really validate field, set error collection and notify validitychange
isValid() - should only inform how error collection looks like (true/false) - It shouldn't validate
getError() - if you want to know something about errors (return error_collection) - It shouldn't validate


I solved the problem by overwriting the Ext.form.Basic, but it's not good way when you plan to use next versions of Extjs in the future.

anyway
Merry Christmas to Everyone

mitchellsimoens
23 Dec 2012, 7:23 AM
I moved this to the bugs forum and I have opened a bug in our bug tracker.

westy
2 Jan 2013, 2:42 AM
Eeek!

I've noticed it getting called a lot, but never actually looked into just how much.