1

I am building simple client side valditation using Judge gem.

Everything worked perfect till I choosed to run validations on "blur" instead of button click.

Script:

var divs = ["advertisement_age","advertisement_weight","advertisement_region_id","advertisement_height","advertisement_name","advertisement_phone_number","Record_Start_Time",
"Record_End_Time","Record_Start_Time2","Record_End_Time2","advertisement_description", "advertisement_description_ru", "advertisement_terms_of_service"];

    var arrayLength = divs.length;
    var error_count =0;
    for (var i = 0; i < arrayLength; i++) {

     $(document.getElementById(divs[i])).blur(function(){

        judge.validate(document.getElementById(divs[i]), {  // This line causes error
                valid: function(element) {
                  element.style.border = '1px solid green';
                },
                invalid: function(element, messages) {
                  element.style.border = '1px solid red';
                 // alert(messages.join(','));

        }

        });

      });

    }

In Mozzila Firebug there isn't any error message when I reload page. Error shows up when blur event is triggered.

Error message:

   TypeError: this.element is null

   this.attrValidators = root.JSON.parse(this.element.getAttribute('data-validate')...

I don't understand why it crashes at second document.getElementById(divs[i]) instead of the first if that array is valid?

Any suggestions ?

Thanks

Edgars
  • 913
  • 1
  • 19
  • 53

1 Answers1

2

The problem is that by the time the blur event fires, the for loop has exited and the value of i will be set to the last value it arrived at when the for loop exited. Then, eventually, when the blur event fires judge.validate(document.getElementById(divs[i]) will fail because i is not what you expected. One way to fix it is by forming a closure over the value of i:

for (var i = 0; i < arrayLength; i++) {
  (function(index) { // index is the current i value
    $(document.getElementById(divs[index])).blur(function() {
      judge.validate(document.getElementById(divs[index]), {
        valid: function(element) {
          element.style.border = '1px solid green';
        },
        invalid: function(element, messages) {
          element.style.border = '1px solid red';
          // alert(messages.join(','));
        }
      });
    });
  })(i); // pass i to the immediately executing anonymous function (closure)
}

So we put the $.blur code inside a closure and pass i in. Inside the closure i will be called index. The value of index will be the expected value at the time the loop executed the code, and will remain that way after the loop exits. Notice how inside the blur handler we're now using divs[index].

The thing to take away from this particular problem is that the blur event handler function is asynchronous and won't evaluate divs[i] until the blur event happens which will be at a time after the for loop has exited and left the value of i at its last value i == arrayLength - 1.

Here's some more reading on that: Calling an asynchronous function within a for loop in JavaScript

Another way to fix this would be to save the element you find with document.getElementById(divs[i]) and then use this inside the handler:

for (var i = 0; i < arrayLength; i++) {
  var field = $(document.getElementById(divs[i]));
  field.blur(function() {
    judge.validate(this, { // `this` is the field element
      valid: function(element) {
        element.style.border = '1px solid green';
      },
      invalid: function(element, messages) {
        element.style.border = '1px solid red';
        // alert(messages.join(','));
      }
    });
  });
}

I'd choose the latter solution as it is cleaner. Just wanted to explain the closure thing first.

Community
  • 1
  • 1
DiegoSalazar
  • 13,361
  • 2
  • 38
  • 55
  • Thanks a lot. I wish I could give more reputation for this. – Edgars Jan 14 '15 at 16:00
  • 1
    Wouldn't that be easer to use `this` instead of `$(document).getElementById(divs[i])` in the handler? Also the current logic will create quite a lot of handlers, it would be more efficient to delegate the event. – BroiSatse Jan 14 '15 at 16:04
  • That's true. I got too excited talking about closures that I neglected to mention that you can just change the `divs` array into a comma separated list of ids (prepending a # to each id) or better yet, give all those divs a class (e.g. `.validateMe`) and just call `$('.validateMe').blur...` which would bind 1 event handler to all the elements at once rendering the loop unnecessary. – DiegoSalazar Jan 14 '15 at 17:17
  • @EdgarsRozenfelds no problem , i'll vote him from your side :P – Mani Dec 10 '15 at 17:10