Trouble evaluating all values of an array in a for loop in Javascript

Question

I'm trying to write my own function to evaluate input fields to see if they have a value. So I've written this for loop:

$("button").click(function(){
    for (var ii = 0; ii < personalInfo.length; ii++) {
      if ($(personalInfo[ii]).val() == 0) {
        $(personalInfoLabels[ii]).addClass("required");
      }
      else{
        $("button").parent("fieldset").slideUp();
      }
    };
});

As of now the loop has no problem identifying the positions of the array that are empty and labeling them correctly.

However, if any of the positions have a value the event happens. How can I say if each position of the loop has a value, fire event, but if not label the required field?

A JSFiddle of the code is available at http://jsfiddle.net/bjKX7/

Sergio · Accepted Answer · 2013-08-16 15:56:34Z

2

I re-arranged your code a bit, check here:

$("#cont-one").click(function () {
    var error = 0;
    for (var ii = 0; ii < personalInfo.length; ii++) {
        if ($(personalInfo[ii]).val() == 0) {
            $(personalInfoLabels[ii]).addClass("required");
            error++;
        } else {
            $(personalInfoLabels[ii]).removeClass("required");
        }
    };
    if (error) {
        return false;
    }
    $("#cont-one").parent("fieldset").slideUp();
});

This code will go thru all inputs and add class to those you wanted. It will also remove that class in case they are filled. I added also a return false; in case there are errors so the function & form submission will stop here (in case of error).

Keep in min that input fields are self closed, so add / closing code to the inputs like I did in the demo.

Demo here

edited Aug 16, 2013 at 15:56

answered Aug 16, 2013 at 15:38

Sergio

28.9k11 gold badges90 silver badges132 bronze badges

Sign up to request clarification or add additional context in comments.

8 Comments

Ian Over a year ago

@FritsvanCampen Agreed. It doesn't highlight all errors, just the first one, which I wouldn't do

Sergio Over a year ago

@Ian, good point also, thanks. Fixed & now it highlights all errors.

Ian Over a year ago

@Sergio Yep, now that you don't return, it allows all elements to be validated :)

Ian Over a year ago

I'm not trying to change your code, but here's how I'd organize it: jsfiddle.net/SznRc - it uses jQuery's native way of iterating over a jQuery object, and kind of simplifies the loop's code. I don't think == 0 was a good choice for checking against, because it will be true if the input is 0

Sergio Over a year ago

@Ian think that is better than OP's also. Using .each is simplifying (thats why jQuery is good to have). I vote up your comment, if you post it as an answer I vote it up also ;)

|

Halcyon · Accepted Answer · 2013-08-16 15:51:02Z

Keep a boolean value outside the loop that is true only when all fields are not empty:

$("button").click(function(){
    var all_good = true;
    for (var ii = 0; ii < personalInfo.length; ii++) {
        if ($(personalInfo[ii]).val() !== "") { // not empty
            $(personalInfoLabels[ii]).addClass("required");
            all_good = false;
        } else {
            $(personalInfoLabels[ii]).removeClass("required"); 
        }
    };
    if (all_good) {
        $("button").parent("fieldset").slideUp();
    }
});

By the way:

$(personalInfo[ii]).val() == 0

works because of coericion, which is a little hard to see. I'd just write:

$(personalInfo[ii]).val() === "" // or
$(personalInfo[ii]).val().length === 0

Because that is what you mean.

values from input fields are always strings, never numbers so you can even use === because you know the type is string. Similarly, String.length is always a number.

SyntaxError: missing ) after argument list. Add a opening curly before // not empty to fix.
Not that it's up to you, but you might want to $.trim() the input value before comparing to "". I know the OP didn't know about it, but it's usually a good suggestion when validating a required field because spaces shouldn't count
And you also might want to keep the else { $(personalInfoLabels[ii]).removeClass("required"); }, otherwise future submits wouldn't clear newly valid fields, right? EDIT: Realized the else I saw was from Sergio's answer, not the OP's code. Still a good idea

techfoobar · Accepted Answer · 2013-08-16 15:37:52Z

0

Validate first and .slideUp() at the end if all is well. Like this

// validate first
for (var ii = 0; ii < personalInfo.length; ii++) {
    if ($(personalInfo[ii]).val() == 0) $(personalInfoLabels[ii]).addClass("required");
}

// check if there are any marked as required, if not, slideUp
var fs = $("#cont-one").parent("fieldset");
if (fs.find('.required').length == 0) fs.slideUp();

answered Aug 16, 2013 at 15:37

techfoobar

66.8k14 gold badges117 silver badges138 bronze badges

Collectives™ on Stack Overflow

Trouble evaluating all values of an array in a for loop in Javascript

3 Answers 3

Demo here

8 Comments

3 Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Demo here

8 Comments

3 Comments

Comments

Related