1
\$\begingroup\$

Well I just finished my first go at my own form validation (I always used validate plugin) for simple front end. It's definitely small potatoes as it's only meant for a form with 4 fields; 3 required text and one required e-mail.

I'm new to Jquery so I am doing little exercises like these to help me learn and I was hoping someone could take a look and see if what I did was ok. Did I make any newbie mistakes? Can I improve this? Perhaps I should use js objects (new to those too) to organize code? I know there is no one answer, in fact I'm sure there are countless ways to validate a form...But I was just hoping some JS expert could offer a little insight on better coding practices.

*EDIT - forgot to mention...I'm aware that I should probably use regex for better email validation, however, just threw the indexOf() method for the time being *

$(function() {

        var err = "<span class='error'>Required field.</span>";
        var errEmail = "<span class='error'>Please enter valid email.</span>";
        var errors = false;

        $('.submit').click(function() {
            $('.error').remove();

            $('.req').each(function() {
                if($(this).val() == '') {
                    $(this).after(err);
                    errors = true;
                } 
            });

            if ($('.reqe').val().indexOf('@') === -1) {
                $('.reqe').after(errEmail);
                errors = true;
            }

            if (errors == true) {
                return false;
            }
            else {
                var errors = false;
                alert("Submitted");
                return true;
            }

        });
});

Any feedback would be greatly appreciated. Thanks

\$\endgroup\$
1
\$\begingroup\$

A couple of small changes:

  1. near the end:

    if (errors == true) {
        return false;
    }
    else {
        var errors = false;
        alert("Submitted");
        return true;
    }
    

    Could be:

    if(!errors)
    {
        alert("Submitted");
    }
    return !errors; 
    
  2. the .req

    if($(this).val() == ''){
    

    could be:

    if(!$(this).val()){
    

    This would actually be a problem because

     " " != ""
    

    so a space would validate (if that's what you intended then ignore this one ;) )

  3. var errors= false; needs to be inside the click event:

     $('.submit').click(function() {
         var errors = false;
    

    otherwise when you click the submit a second time after an invalid attempt it won't reset the value.

Alternatively:-

http://jsfiddle.net/ssTPm/

var validationConfig = [
{
    "selector": ".req",
    "hasErrors": function() {
        return !$(this).val()
    },
    "errorMessage": "<span class='error'>Required field.</span>"},
{
    "selector": '.reqe',
    "hasErrors": function() {
        return $('.reqe').val().indexOf('@') === -1
    },
    "errorMessage": "<span class='error'>Please enter valid email.</span>"
}];

then a nice loop like:

for (var i = 0, validation = null; validation = validationConfig[i]; i++) {
    $(validation.selector).each(validateEach);
}

and validateEach would look like:

 function validateEach () {
    var hasErrors = validation.hasErrors.call(this);
    errors |= hasErrors;
    if (hasErrors) {
        $(this).after(validation.errorMessage);
    }
};

This would mean you could have the validation function in you js files and then when you initialise the page from the server you just output the validationConfig:

validationConfig = validationConfig || [];
validationConfig.concat([
    {
        "selector": ... 
    }
];

as many times as you like. Although from the looks of things it might be over-engineering things a bit.

Explanations:-

so the || is a coalesce or OR operator.

myVariable = myVariable || [];

means if myVariable results in a true value (not false, undefined, null etc etc) then it does nothing otherwise it assigns it a defualt value.

so if I had the following:

myList = myList|| [];
myList.push(1);
myList = myList|| [];
myList.push(2);
myList = myList|| [];
myList.push(3);

it would result in [1,2,3]. The first time it sets the value as [] the next times they just assign itself to itself. This way it doesn't need to know if it has been initialised or not.

the |= is a bitwise | (BITWISE OR) and an = so

value |= otherValue;

is the equivalent to

value = value | otherValue;

In this example

errors |= hasErrors;

is the exact same as

errors = errors || hasErrors;

Errors is assigned true as long as either it was already true or hasErrors is true;

\$\endgroup\$

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.