JavaScript Validation

Bit of a preface on my motive: I'm self taught and have never really worked in a collaborative environment. My only benchmark for quality is "does it work" and I've never been really subject to peer review.

With that in mind I just thought I'd dump a small jQuery validation plugin I wrote here and see if anyone can spot any rookie errors or advise on best practice. I'm very aware that because I've never had anyone looking over my shoulder while I'm coding, I could be committing all sorts of programmatic faux pas and not even realising.

Here's the plugin in its entirety followed by some example markup (sorry about the annoying comments)

Demo Link

```
// Using a global "Tiga" object. This allows me to extend the same object with all my different plugins

if (typeof Tiga == "undefined") {
Tiga = {};
}

//extend jQuery with the .tigaValidate method. This allows for easy running
$.fn.tigaValidate = function(options) {
// Plugin Settings
var s = $.extend( {
// Default Settings
form : this,
live : true,
formatting : true,
invalid : function($form){
$("body, html").animate({
scrollTop: 0
}, 500);
$form.addClass("error");
}
}, options);

//let's just call our the init() method of our core object now.
Tiga.Validate.init(s.form, s.live, s.formatting, s.invalid);
};

// here's our core object
Tiga.Validate = {
init: function(form, live, formatting, invalid){
// defaults (for when/if Tiga.Validate.init() is invoked directly)
if (typeof live == "undefined") { live = true; };
if (typeof formatting == "undefined") { formatting = true; };
// cache $(form)
var $form = $(form),

Sure, I can give you some pointers. Firstly you mention about the 'annoying' comments, well if you think they're annoying then what use do they have to another person who would be reading the code? I think comments should not really describe line by line what something does, only in exceptional cases where the code looks weird; point out that you're doing something because of a browser bug etc. Adding a comment such as Initialize before a declaration isn't going to help you or a future maintainer.

You can make some improvements to readability when using return. For example:

// return validity check
if (valid) {
    return true;
} else {
    return false;
};

Can simply be this:

return valid;

Similarly:

//validation check on form and enable/disable the button;
if (Tiga.Validate.validateForm(form)) {
    $submit.attr("disabled", false);
} else {
    $submit.attr("disabled", true);
};

Becomes this:

$submit.attr('disabled', !Tiga.Validate.validateForm(form));

As a side note to this point, the final semi-colon in the block isn't necessary. Consider running a tool such as JSHint on your code. As you can see, pasting your code there will give you a ton of suggestions for improvement.

Test against booleans. This line:

if ($(o).attr("data-invalid") != undefined || $(o).val().length == 0) {

Should be:

if ($(o).attr("data-invalid") || !$(o).val().length) {

In that you are testing for a data-invalid attribute (truthy) or that the length is falsey (0).

Performance: Always try to cache the jQuery lookups when you use a selector multiple times in the same block. It's good for minifiers too because something like var $myDiv = $('#myDiv'); can be condensed to var e = $('#myDiv'); and then referred to as e instead of writing the selector string again. An example in your code:

var val = $(this).val().toUpperCase();          
$(this).val(val);

Can be this:

var $this = $(this);
$this.val($this.val().toUpperCase());

And in this case you don't need to cache val in a variable because it's only used in that single place.

Code formatting; try and use braces for your else blocks, it enhances readability:

setCaretPos: function(field, caretPos) {
    if(field != null) {
        if(field.createTextRange) {
            var range = field.createTextRange();
            range.move('character', caretPos);
            range.select();
        }
        else {
            if(field.selectionStart) {
                field.focus();
                field.setSelectionRange(caretPos, caretPos);
            }
            else
                field.focus();
        }
    }
}

Will look much neater like this:

setCaretPos: function(field, caretPos) {
    if (field !== null) {
        if (field.createTextRange) {
            var range = field.createTextRange();
            range.move('character', caretPos);
            range.select();
        } else {
            if (field.selectionStart) {
                field.focus();
                field.setSelectionRange(caretPos, caretPos);
            } else {
                field.focus();
            }
        }
    }
}

It can also be tidied up further:

// ...
        } else {
            field.focus();
            if (field.selectionStart) {
                field.setSelectionRange(caretPos, caretPos);
            }
        }

Finally, this is a validation plugin so I expected some visual feedback that I was doing something wrong; perhaps a message by each field or even just a red/green border to indicate validity. I'd suggest that some feedback to the user here would make the form more pleasant to fill out.

Hope this helps!

StackExchange Code Review Q#62831, answer score: 5