views:

24

answers:

1

Hi everyone!

I've got this validation script for my form (a survey). It loops through all answers, and checks if it is a radiobutton, a checkbox, input or textarea question. Based on that, it respectively checks for an input with the value checked (both radio and checkbox), or a value other then "" (input/textarea) (it's not that much, but it is something to start with). The requiredMessage() sets a warning if fields are left empty, or removes the warning when a field is corrected by the user.

I'm looking for a way to optimize this code because it looks very repetative, but I'm not too sure how to begin. Although the blocks have some things in common, they don't seem to fit into one, generic check. Any help is welcome!

Note: I don't expect a fully reviewed version, I just need some direction in where to look for improvements on this, or how to use jQuery to chain those validation-checks together.

Here's the javascript:

// validate the form
function validate(event){
    var sendform = true;
  $(".questionnaire-question-answer").each(function(){

// radiobutton case
    if($(this).find("input:radio").length){
        if($(this).find("input:radio:checked").length == 0){
            requiredMessage(this, "invalid");
            sendform = false;
        } else {
            requiredMessage(this, "valid");
        }
    }
// checkboxes case
    else if($(this).find("input:checkbox").length){
         if($(this).find("input:checkbox:checked").length == 0){
            requiredMessage(this, "invalid");
            sendform = false;
         } else {
            requiredMessage(this, "valid");
         }
    }
// single freetext case
    else if($(this).find("input:text").length == 1) {
         if($(this).find("input:text").val() == ""){
            requiredMessage(this, "invalid");
            sendform = false;
         } else {
            requiredMessage(this, "valid");
         }
    }
// multiple freetext case
    else if($(this).find("input:text").length > 1) {
        var content = false;
        $(this).find("input:text").each(function(){
            if($(this).val() != ""){
              content = true;
              requiredMessage(this, "valid");  
            }
        });
        if(content == false){
          requiredMessage(this, "invalid");
          sendform = false;
        }
    }
// textarea case
    else if($(this).find("textarea").length){
         if($(this).find("textarea").val() == ""){
           requiredMessage(this, "invalid");
           sendform = false;
         } else {
           requiredMessage(this, "valid");
         }
    }
  });
// check whether the form has errors
  if(sendform == true){
    return true;
  } else {
    event.preventDefault();
  }
}
+1  A: 

You can narrow it down quite a bit with selectors, this is very close but not exactly what you have for logic, give it a try:

function validate(event){
  var sendform = true;
  $(".questionnaire-question-answer").each(function() {
    var e = $(this).find("input:checked,input:text[value!=''],textarea[value!='']");
    //e is input/textarea elements with values
    if(e.length) {
      requiredMessage(this, "valid");
    } else {
      requiredMessage(this, "invalid");
      sendform = false;
    }
  });

  if(sendform) return true;
  event.preventDefault();
}
Nick Craver
@Nick thanks, that looks way more to what I want. I'll give it a shot and come back on this!
Justus Romijn
@Justus - needed one tweak, the addition of the `:text` or `[type=text]` selector to the input with a value check, make sure to get that update when testing.
Nick Craver
It works like a charm! I have a hard time thinking more abstract, but I'm learning step by step :)
Justus Romijn
By the way: It reduced the code by at least 40 lines. Yay!
Justus Romijn