5
\$\begingroup\$

I am trying to create a simple validation for forms. I am almost there, but there are few bugs and optimization errors that I am really struggling with. I'm looking for advice on how to make this function simpler and more understandable.

var errorMessage = {
   required: "This field can not be empty",
   email: "Please enter a valid email address",
   number: "Please only enter numbers in this field",
   min: "This field should be minimum ",
   max: "This field should be maximum ",
   date: "Please use the date format outlined above"
};

var $form = $("#contactForm");
var $formInputs = $("#contactForm input");
var $formTextareas = $("#contactForm textarea");
var $formSelects = $("#contactForm select");
var currentRadioSet = "";

$form.submit(function(event) {
   event.preventDefault();
   if (submitValidate()) {
      var $el = $(this);
      $submit = $el.find('button[id="submit"]');
      $inputs = $el.find('input, textarea, select, label');
      $.post($el.attr('action'), $form.serialize(), function(data) {
         $('span.error').remove();
         if (data) {
            $submit.text('Sent. Thank You!');
            $submit.add($inputs).addClass('disabled').prop('disabled', true);
         } else {
            $submit.after('<span style="display: inline-block; padding: 15px 5px; color: #bd3d3d">Failed to send the message, please try again later.</span>');
            $submit.text('Try Again');
         }
      });
   }
});

$formInputs.keyup(function() {
   var $el = $(this);
   if (!$el.hasClass("required")) {
      resetSingleErrorMessage($el);
   }
   checkForClasses($el);
});

$formTextareas.keyup(function() {
   var $el = $(this);
   if (!$el.hasClass("required")) {
      resetSingleErrorMessage($el);
   }
   checkForClasses($el);
});

$formTextareas.blur(function() {
   var $el = $(this);
   if (!$el.hasClass("required")) {
      resetSingleErrorMessage($el);
   }
   checkForClasses($el);
});

$formInputs.blur(function() {
   var $el = $(this);
   if (!$el.hasClass("required")) {
      resetSingleErrorMessage($el);
   }
   checkForClasses($el);
});

function submitValidate() {
   resetErrorMessages();

   $formInputs.each(function() {
      var $el = $(this);
      checkForClasses($el);
   });

   $formTextareas.each(function() {
      var $el = $(this);
      checkForClasses($el);
   });

   $formSelects.each(function() {
      var $el = $(this);
      checkForClasses($el);
   });

   if (!$('.error').length) {
      return true;
   } else {
      return false;
   }
}

function checkForClasses(elementIn) {
   var $el = $(elementIn);
   var isValid = true;
   isValid = checkForMin($el);
   if (!isValid) {
      return;
   }
   isValid = checkForMax($el);
   if (!isValid) {
      return;
   }
   if ($el.hasClass("required")) {
      isVaild = requiredVal($el);
      if (!isValid) {
         return;
      }
   }
   if ($el.hasClass("email")) {
      isVaild = emailVal($el);
      if (!isValid) {
         return;
      }
   }
   if ($el.hasClass("number")) {
      isVaild = numberVal($el);
      if (!isValid) {
         return;
      }
   }
   if ($el.hasClass("date")) {
      isVaild = dateVal($el);
      if (!isValid) {
         return;
      }
   }
}

function requiredVal(elementIn) {
   var $el = $(elementIn);
   var valid = true;
   if ($el.is(':radio:last') || $el.is(':checkbox:last')) {
      currentRadioSet = $el.attr("name");
      valid = $("input[name=" + currentRadioSet + "]:checked").val();
   }

   if ($.trim($el.val()) == "" || !valid) {
      manageErrorMessage($el, errorMessage.required);
      valid = false;
   } else {
      resetSingleErrorMessage($el);
      valid = true;
   }


   return valid;
}

function numberVal(elementIn) {
   var $el = $(elementIn);
   var valid = true;
   var numRegEx = /^\d+$/;
   var numIn = $.trim($el.val());
   if (!$.trim($el.val()) == "") {
      if (!numRegEx.test(numIn)) {
         manageErrorMessage($el, errorMessage.number);
         valid = false;
      } else {
         resetSingleErrorMessage($el)
         valid = true;
      }
   }
   return valid;
}

function emailVal(elementIn) {
   var $el = $(elementIn);
   var valid = true;
   var emailRegEx = /^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,4}$/;
   var emailIn = $.trim($el.val());
   if (!$.trim($el.val()) == "") {
      if (!emailRegEx.test(emailIn)) {
         manageErrorMessage($el, errorMessage.email);
         valid = false;
      } else {
         resetSingleErrorMessage($el)
         valid = true;
      }
   }
   return valid;
}

function dateVal(elementIn) {
   var $el = $(elementIn);
   var valid = true;
   var dateRegEx = /^(0?[1-9]|[12][0-9]|3[01])[\/\-](0?[1-9]|1[012])[\/\-]\d{4}$/;
   var dateIn = $.trim($el.val());
   if (!$.trim($el.val()) == "") {
      if (!dateRegEx.test(dateIn)) {
         manageErrorMessage($el, errorMessage.date);
         valid = false;
      } else {
         resetSingleErrorMessage($el);
         valid = true;
      }
   }
   return valid;
}

function checkForMin(elementIn) {
   var $el = $(elementIn);
   var valid = true;
   if ($el.attr("class")) {
      if ($el.attr("class").match(/min[0-9]+/)) {
         var minClass = $el.attr("class").match(/min[0-9]+/).toString();
         minVal = parseInt(minClass.match(/[0-9]+/));
         if ($.trim($el.val().length) < minVal) {
            manageErrorMinMax($el, errorMessage.min, minVal);
            valid = false;
         } else {
            resetSingleErrorMessage($el);
            valid = true;
         }
      }
   }
   return valid;
}

function checkForMax(elementIn) {
   var $el = $(elementIn);
   var valid = true;
   if ($el.attr("class")) {
      if ($el.attr("class").match(/max[0-9]+/)) {
         var maxClass = $el.attr("class").match(/max[0-9]+/).toString();
         maxVal = parseInt(maxClass.match(/[0-9]+/));

         if ($.trim($el.val().length) > maxVal) {
            manageErrorMinMax($el, errorMessage.max, maxVal);
            valid = false;
         } else {
            resetSingleErrorMessage($el);
            valid = true;
         }
      }
   }
   return valid;
}

function manageErrorMessage(elementIn, errorMessageIn) {
   var $el = $(elementIn);
   $el.addClass('error');
   if (!$el.next().is("span")) {
      $el.after('<span class="error"></span>');
   }
   $el.next().html(errorMessageIn);
}

function manageErrorMinMax(elementIn, errorMessageIn, value) {
   var $el = $(elementIn);
   $el.addClass('error');
   if (!$el.next().is("span")) {
      $el.after('<span class="error"></span>');
   }
   $el.next().html(errorMessageIn + value + " charaters long.");
}

function resetSingleErrorMessage(elementIn) {
   var $el = $(elementIn);
   $el.removeClass("error");
   if ($el.next().is("span")) {
      $el.next().remove();
   }
}

function resetErrorMessages() {
   $("span.error").remove();
   $("input .error").removeClass("error");
}
  1. The script is really ugly and not optimized at all which is what causing some problems as I can't just understand what is what and which is which, therefore I am now trying to clean it up. I am not an expert in JavaScript but have basic knowledge - the script is the results of a very long nights researching and collecting things and putting them together...

    A) I cleaned up this bit:

    var $form = $("#contactForm");
    var $formInputs = $("#contactForm input");
    var $formTextareas = $("#contactForm textarea");
    var $formSelects = $("#contactForm select");
    var currentRadioSet = "";
    

    so now it appears like this:

    var $form = $("form");
    var $inputs = $("input, textarea, select, label");
    

    B) I have accompanied all the key-up and blur together "This is for a feel of a real-time validation":

    $inputs.keyup(function () {
        var $element = $(this);
        if (!$element.hasClass("required")) {
            resetSingleErrorMessage($element);
        }
        checkForClasses($element);
    });
    
    $formTextareas.keyup(function () {
        var $element = $(this);
        if (!$element.hasClass("required")) {
            resetSingleErrorMessage($element);
        }
        checkForClasses($element);
    });
    
    $formTextareas.blur(function () {
        var $element = $(this);
        if (!$element.hasClass("required")) {
            resetSingleErrorMessage($element);
        }
        checkForClasses($element);
    });
    
    $inputs.blur(function () {
        var $element = $(this);
        if (!$element.hasClass("required")) {
            resetSingleErrorMessage($element);
        }
        checkForClasses($element);
    });
    

    So now they appear like this:

    $inputs.bind('blur keyup focus', function(){
        var $element = $(this);
        if (!$element.hasClass("required")) {
            resetSingleErrorMessage($element);
        }
        checkForClasses($element);
     });
    

    C) I also cleaned up this section that I believe it validates on submit:

    function validate() {
        resetErrorMessages();
    
        $inputs.each(function () {
            var $element = $(this);
            checkForClasses($element);
        });
    
        $formTextareas.each(function () {
            var $element = $(this);
            checkForClasses($element);
        });
    
        $formSelects.each(function () {
            var $element = $(this);
            checkForClasses($element);
        });
    
        if (!$('.error').length) {
            return true;
        } else {
            return false;
        }
    }
    

    So it appear like this now:

    function validate() {
        resetErrorMessages();
        $inputs.each(function () {
            var $element = $(this);
            checkForClasses($element);
        });
        if (!$('.error').length) {
            return true;
        } else {
            return false;
        }
    }
    

    D I have also cleaned this bit up, this section check if the inputs has any classes and then pass it to validate.

    function checkForClasses(elementIn) {
        var $el = $(elementIn);
        var isValid = true;
        isValid = checkForMin($el);
        if (!isValid) {
            return;
        }
        isValid = checkForMax($el);
        if (!isValid) {
            return;
        }
        if ($el.hasClass("required")) {
            isVaild = requiredVal($el);
            if (!isValid) {
                return;
            }
        }
        if ($el.hasClass("email")) {
            isVaild = emailVal($el);
            if (!isValid) {
                return;
            }
        }
        if ($el.hasClass("number")) {
            isVaild = numberVal($el);
            if (!isValid) {
                return;
            }
        }
        if ($el.hasClass("date")) {
            isVaild = dateVal($el);
            if (!isValid) {
                return;
            }
        }
    }
    

    It is now appearing like:

    //Check for validation rules in inputs
    function checkForClasses(element) {
        var $element = $(element);
        var valid = true;
    
    //check for required
    if ($element.hasClass("required")) {
        valid = Required($element);
        if (!valid) {
            return;
        }
    }
    //check for email
    if ($element.hasClass("email")) {
        valid = Email($element);
        if (!valid) {
            return;
        }
    }
    //check for number
    if ($element.hasClass("number")) {
        valid = Number($element);
        if (!valid) {
            return;
        }
    }
    //check for minimum
    valid = Minimum($element);
    if (!valid) {
        return;
    }
    //check for maximum
    valid = Maximum($element);
    if (!valid) {
        return;
    }
    

    }

Why does Man and Min not work if I did them as all the others? If I used this method, they won't work.

if ($element.hasClass("max")) {
           valid = Maximum($element);
          if (!valid) {
             return;
          }
       }

Is there any way to clean this section farther and make it simpler? The form has to be dynamic and I can't seem to be able to figure out away to make it check for classes dynamically without telling it where to go manually.

I just don't know what to do with the rest of the code. Every time I try to optimize it, it breaks. Please can someone help me?

Some of the bugs: I can't validate radio and checkbox buttons, and the validation for the select input only works when submit not in a real time.

Here is the final code after cleaning up:

$(document).ready(function () {
if ($('form').length > 0) {
var errorMessage = {
    required:  "This field can not be empty",
    email:     "Please enter a valid email address",
    number:    "This field can only contain numbers",
    min:       "This field should be minimum ",
    max:       "This field should be maximum ",
};
var $form = $("form");
var $inputs = $("input, textarea, select, label");
$form.submit(function (event) {
   event.preventDefault();
   if (validate()) {
   var $element = $(this);
   $submit = $element.find('button[id="submit"]');
   $inputs = $element.find('input, textarea, select, label');
   $.post($element.attr('action'), $form.serialize(),
      function (data) {
         $('span.error').remove();
         if (data) {
            $submit.text('Sent. Thank You!');
            $submit.add($inputs).addClass('disabled').prop('disabled', true);
         } else {
            $submit.text('Try Again');
         }
      });
   }
});
//Check for validation rules in inputs
function checkForClasses(element) {
   var $element = $(element);
   var valid = true;
   //check for required
   if ($element.hasClass("required")) {
       valid = Required($element);
      if (!valid) {
         return;
      }
   }
   //check for email
   if ($element.hasClass("email")) {
       valid = Email($element);
      if (!valid) {
         return;
      }
   }
   //check for number
   if ($element.hasClass("number")) {
       valid = Number($element);
      if (!valid) {
         return;
      }
   }
   //check for minimum
   valid = Minimum($element);
   if (!valid) {
      return;
   }
   //check for maximum
   valid = Maximum($element);
   if (!valid) {
      return;
   }
}
//Real-time validation
$inputs.bind('blur keyup focus', function(){
   var $element = $(this);
   if (!$element.hasClass("required")) {
      resetSingleErrorMessage($element);
   }
   checkForClasses($element);
});
//Validate on submit
function validate() {
   resetErrorMessages();
   $inputs.each(function () {
      var $element = $(this);
      checkForClasses($element);
   });
   if (!$('.error').length) {
      return true;
   } else {
      return false;
   }
}
//Required - validation rule
function Required(element) {
    var $element = $(element);
    var valid = true;
    if ($element.is(':radio:last') || $element.is(':checkbox:last')) {
        currentRadioSet = $element.attr("name");
        valid = $("input[name=" + currentRadioSet + "]:checked").val();
    }
    if ($.trim($element.val()) == "" || !valid) {
        manageErrorMessage($element, errorMessage.required);
        valid = false;
    } else {
        resetSingleErrorMessage($element);
        valid = true;
    }
    return valid;
}
//Email - validation rule
function Email(element) {
    var $element = $(element);
    var valid = true;
    var emailRegEx = /^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,4}$/;
    var emailIn = $.trim($element.val());
    if (!$.trim($element.val()) == "") {
        if (!emailRegEx.test(emailIn)) {
            manageErrorMessage($element, errorMessage.email);
            valid = false;
        } else {
            resetSingleErrorMessage($element)
            valid = true;
        }
    }
    return valid;
}
//Number - validation rule
function Number(element) {
    var $element = $(element);
    var valid = true;
    var numRegEx = /^\d+$/;
    var numIn = $.trim($element.val());
    if (!$.trim($element.val()) == "") {
        if (!numRegEx.test(numIn)) {
            manageErrorMessage($element, errorMessage.number);
            valid = false;
        } else {
            resetSingleErrorMessage($element)
            valid = true;
        }
    }
    return valid;
}
//Minimum - validation rule
function Minimum(element) {
    var $element = $(element);
    var valid = true;
    if ($element.attr("class")) {
        if ($element.attr("class").match(/min[0-9]+/)) {
            var minClass = $element.attr("class").match(/min[0-9]+/).toString();
            minVal = parseInt(minClass.match(/[0-9]+/));
            if ($.trim($element.val().length) < minVal) {
                manageErrorMinMax($element, errorMessage.min, minVal);
                valid = false;
            } else {
                resetSingleErrorMessage($element);
                valid = true;
            }
        }
    }
    return valid;
}
//Maximum - validation rule
function Maximum(element) {
    var $element = $(element);
    var valid = true;
    if ($element.attr("class")) {
        if ($element.attr("class").match(/max[0-9]+/)) {
            var maxClass = $element.attr("class").match(/max[0-9]+/).toString();
            maxVal = parseInt(maxClass.match(/[0-9]+/));
            if ($.trim($element.val().length) > maxVal) {
                manageErrorMinMax($element, errorMessage.max, maxVal);
                valid = false;
            } else {
                resetSingleErrorMessage($element);
                valid = true;
            }
        }
    }
    return valid;
}
//Show and hide error messages
function manageErrorMessage(element, errorMessageIn) {
    var $element = $(element);
    $element.addClass('error');
    if (!$element.next().is("span")) {
        $element.after('<span class="error"></span>');
    }
    $element.next().html(errorMessageIn);
}
function manageErrorMinMax(element, errorMessageIn, value) {
    var $element = $(element);
    $element.addClass('error');
    if (!$element.next().is("span")) {
        $element.after('<span class="error"></span>');
    }
    $element.next().html(errorMessageIn + value + " charaters long.");
}
function resetSingleErrorMessage(element) {
    var $element = $(element);
    $element.removeClass("error");
    if ($element.next().is("span")) {
        $element.next().remove();
    }
}
function resetErrorMessages() {
    $("span.error").remove();
    $("input .error").removeClass("error");
}
}
});

Here is the HTML of the form:

   <form action="contact.php" id="contactForm" method="post" name="contactForm">

      <div class="name">
         <p>Your Name</p>
         <input class="required" name="name">
      </div>

      <div class="email">
         <p>Email Address</p>
         <input class="required email" name="email">
      </div>

      <div class="number">
         <p>Number</p>
         <input class="required number" name="number">
      </div>

      <div class="message">
         <p>Message</p>
         <textarea class="required min5 max10" name="message" rows="5"></textarea>
      </div>

      <div class="dropdown">
         <select class="required">
            <option value="">Select</option>
            <option value="option">Option</option>
         </select>
      </div>

      <div class="radio">
         <p>Radios:</p>
         <label><input name="sex" type="radio" value="value">radio</label> 
         <label><input name="sex" type="radio" value="value">radio</label>
      </div>

      <div class="checkbox">
         <p>Checkboxs:</p>
         <label><input name="checkbox" type="checkbox" value="value">checkbox</label> 
         <label><input name="checkbox" type="checkbox"value="value">checkbox</label>

      </div><button id="submit" type="submit">Send</button>
   </form>
\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

From a once over:

  • Function names in JavaScript should only start with a capital if they are constructors, so Number is no good, since it returns a boolean, I would call it isNumber()
  • I would put a "use strict"; after $(document).ready(function () {
  • if (!$.trim($element.val()) == "") { should be
    if ($.trim($element.val()) != "") { ( put the ! in the comparison ).
  • $submit, currentRadioSet, minVal and maxVal were not declared with var
  • From a design perspective, it does not seem right that your validation rules manage errors and can reset error messages, in my mind, that should be done elsewhere (checkForClasses?), this will reduce a lot copy of paste code
  • For Minimum and Maximum, I would advise to get and set the minimum/maximum throught the $().data() mechanism instead of classnames and regexes.
  • In checkForClasses you already do var $element = $(element); and then in each validation function you do again var $element = $(element);, that is overkill.
  • I would only check for minimum and maximum if the Number class is present
  • Use either single or double quotes for strings, dont mix, ideally use single quotes
  • In validate() you can simply
    return !$('.error').length;
  • Also, you are again doing var $element = $(this); in validate() :D
  • Please use jshint.com to self review your code, a lot of this feedback comes from there.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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