Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've got some pretty simple validation that I'm working on, but it seems like I'm repeating myself a bunch of times. There has to be a better way of doing this. I'm simply checking if there is a value filled in for specific fields and apply a color if the value isn't filled in. There are also other areas where the validation is different where I'm checking a valid phone number. So I couldn't simply apply this to all inputs. Here's some of my code:

var x = $();
var pass = true;

function validateForm(){
        x = $('input#zip').val();
        if (x == null || x == "") {
            $('#ziplabel').css('color','#fff');
            pass = false;
        } else {
            $('#ziplabel').css('color','#444');
        }
        x = $('input#fname').val();
        if (x == null || x == "") {
            $('#fnamelabel').css('color','#fff');
            pass = false;
        } else {
            $('#fnamelabel').css('color','#444');
        }
        x = $('input#lname').val();
        if (x == null || x == "") {
            $('#lnamelabel').css('color','#fff');
            pass = false;
        } else {
            $('#lnamelabel').css('color','#444');
        }

}
share|improve this question

1 Answer 1

up vote 5 down vote accepted

Yup, you can iterate the input fields. For this sort of thing you should also be using classes, instead of inline CSS. Using jQuery .each and .togggleClass you can create something like this:

function validate () {

  $('#form input').each(function () {
     var $el = $(this)
     $el.prev('label').toggleClass('invalid', $el.val()); 
  });

}

And your stylesheet has a rule like this;

label {
  color: #444;
}

label.invalid {
  color: #fff;
}

If you want to check if the form was valid then you can check for the existence of a field with that class. $('#form input.invalid').length. Or you can set a boolean in the each function if you want to separate the logic from the HTML.

share|improve this answer
1  
You could avoid the caching line if you use $(this).prev('label').toggleClass('invalid',this.value) –  elclanrs Feb 7 at 4:46

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.