2
\$\begingroup\$

I would really like to hear how I can improve my form validator. Next, I want to refactor it into OO and learn a testing framework like Jasmine.

Some things I know I could improve:

  • Don't make it tied to the DOM as much (using parent().parent(), for ex.).
  • Combined validations can't be used. For example, using isBlank and isInteger will not validate properly because if isInteger fails, it throws an error and isBlank passes and clears all errors.
  • Make a config object to update with custom error messages.
  • Possibly make a jQuery plugin so passing the object to validate is easier.

Github repo

  jQuery -> # on Document ready

    errorCount = null

    valEl = $('.validate')  # cache .validate elements
    submitBtn = valEl.parent().parent().find('input[type=submit]')

    valEl.on "blur", ->
      input = $(this)
      validate(input)

    validate = (input) ->
      inputVal = input.val()
      inputLen = inputVal.length
      validations = input.data()         #cache data-xxx tags

      if validations.notblank is on
        if inputLen <= 0
          throwError("This field can't be blank", input)
        else
          removeError(input)

      if validations.minchars > 0
        if inputLen < validations.minchars
          throwError("You need at least 5 chars", input)
        else
          removeError(input)

      if validations.isinteger is on
        isInteger = /^\d+$/
        if isInteger.test(inputVal) is false
          throwError("Must be a whole number", input)
        else
          removeError(input)

    throwError = (message, input) ->
      if !input.hasClass('has-error')
        errorCount += 1
        input.addClass "has-error"
        input.nextAll('.errorContainer').append(message)

    removeError = (input) ->
      if input.hasClass('has-error')
        errorCount -= 1
        input.removeClass("has-error")
        input.nextAll('.errorContainer').empty()

    submitBtn.on "click", (e) ->
      e.preventDefault()
      if errorCount is 0
        valEl.parent().parent().submit()
\$\endgroup\$
1
\$\begingroup\$

You could break out your validation functions into a separate object, like so:

validators =
  isinteger: (input) ->
    # check that value is an int

  notblank: (input) ->
    # check that value is present

These functions could return a simple boolean, or they could return an error message if validation fails and null if it passes.

The point is that you keep the functions separate, so they're easier to manage and customize. But it also means that you can give your inputs data attributes like data-validator which simply is the name of the validation method to use. So you can say, for example

method = $(input).data "validator"
result = validators[method]?(input)

With a little string-splitting, you could make the field plural, like data-validators="isinteger, notblank", and check each.

You could also make the error-throwing into an event you can listen for elsewhere. Then you avoid having your error presentation mixed up with your validations, and you don't have to rely on a predefined .errorContainer element. For instance:

throwError = (message, input) ->
  $(input).trigger 'validation:error', message

# ... elsewhere ...

$(document).on 'validation:error', 'input', (event) ->
  # handle error event

Other stuff I noticed:

  1. Don't bind to the submit button's click event! Forms can also be submitted by simply pressing enter/return, without ever clicking the submit button. Instead, bind the form element's submit event which is fired regardless of how the form is submitted, and use preventDefault() to stop the submission if necessary.

  2. Your notblank logic won't complain if the user just enters some space characters. In most cases the field should probably be considered blank, if it only contains whitespace. So use $.trim() on the value first, to trim off leading and trailing whitespace.

  3. HTML5 has a required attribute you can add to inputs - no need to have your own syntax for say an input should be notblank. Check for the required attribute instead (if $(input).attr('required') ...). It makes the HTML more semantically correct.


I don't know if you already have solutions for the specific things you mention, but here are my ideas:

  • You can use .closest("form") to find the form element instead of parent().parent()

  • If you store all validation failures in an array, you just need to check the array's length, to see if the were any errors. Multiple validations won't override each other, and you don't need errorCount

\$\endgroup\$
  • \$\begingroup\$ Wow thanks! I can't wait to start refactoring! \$\endgroup\$ – SkinnyGeek1010 Oct 16 '12 at 20:24
1
\$\begingroup\$

One thing you could consider doing is following the approach set out by @Flambino with different Validator objects. You could start with a ValidatorBase object that you can decorate with additional methods or even override existing methods with new definitions (see Javascript Decorator pattern at: http://www.joezimjs.com/javascript/javascript-design-patterns-decorator/).

Here's a really crude/untested example of what I'm getting at:

var ValidatorBase = function() {
    console.log('Booya baby1');
}

ValidatorBase.prototype = {
    validate: function() {
        console.log('ValidatorBase::validate()');
    }
}

ValidatorDecorator = function( objValidator ){
    this.validator = objValidator;

}

ValidatorDecorator.prototype = {
    validate: function( objElement ) {
        console.log('ValidatorDecorator::validate()');
        return ( objElement && objElement.value !=  '' );
    }
}

DateValidator = function( objValidator ){
    ValidatorDecorator.call( this, objValidator );
}

DateValidator.prototype = new ValidatorDecorator();
DateValidator.prototype.checkDate = function( date ) {
    return date.match(/^(?:(0[1-9]|1[012])[\- \/.](0[1-9]|[12][0-9]|3[01])[\- \/.](19|20)[0-9]{2})$/);
}
DateValidator.prototype.validate = function( objElement ) {
    console.log('DateValidator::validate()');
    return (/* validation logic, returns True|False*/ this.validator.validate( objElement ) && ( this.checkDate( objElement.value ) ) );
}

Personally, I'd refactor this and wrap it into a module or plugin (same thing in jQuery land.)

This approach let's you take advantage of jQuery's event bindings while providing you with the ability to consistently apply different types of validators to the types of elements/data you want to validate. You may still want to map fields to validators types, or use the data-attribute, e.g. <input type="text" data-validator-type="DateValidator" /> to specify which validator gets applied (this can be done in a single validate() method or you can handle it during serialization of your data if you're using AJAX... or hook the submit event on your form.)

\$\endgroup\$
  • \$\begingroup\$ Cool, i'll have to read up on the decorator pattern! \$\endgroup\$ – SkinnyGeek1010 Oct 16 '12 at 20:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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