[code review] form validation

here is a code i recently did

I would be very interested to see possible improvements!:slight_smile:

Its a javascript



function validateForders(form)//one ring to rule them all
{
	var valid = true;
	
	if (nBlank()=== false)
		{
			valid= false;	
		}
	if (cDefault()=== false)
		{
			valid= false;
		}
	if (numVal()=== false)
		{
			valid = false;
		}
	if (valEmail()===false)
		{
			valid = false;
		}
	if (radioVal()===false)
		{
			valid = false;
		}
	if (delDrops()===false)
		{
			valid = false;
		}
	if (checkText()===false)
		{
			valid = false;
		}
		return valid;
		
}



function nBlank()// blank fields
{
	if (document.Forders.fName.value.length ==0 ||document.Forders.fName.value == "Please enter your name")
		{
		alert("Please enter your name");
		return false;
		}
		
	if (document.Forders.address.value.length ==0 ||document.Forders.address.value == "Please enter your address" )
		{
		alert("Please enter your address");
		return false;
		}
	if (document.Forders.suburb.value.length ==0||document.Forders.suburb.value == "Suburb" )
		{
		alert("Please enter your suburb");
		return false;
		}
	
}
function cDefault() // default drop downs
{
	if(document.Forders.aState.value == "000")
		{
		alert("Please select a State");
		return false;
		}
	if(document.Forders.cCard.value == "000" )
		{
		alert("Please choose your card type");
		return false;
		}
	if(document.Forders.emonth.value == "000" )
		{
		alert("Please select card expiry month");
		return false;
		}
	if(document.Forders.eyear.value == "000" )
		{
		alert("Please select card expiry year");
		return false;
		}
	if(document.Forders.basket.value == "000" )
		{
		alert("Please make a basket choice");
		return false;
		}
	if(document.Forders.quan.value == "000" )
		{
		//alert("You have chosen 1 basket");
		//return false;
		}
}




function numVal() //number validation master
{
	var pc = document.Forders.postcode.value;
	if(isBlank(pc) || isNum(pc) || pc.length != 4)
		{
		alert("Please enter a valid Post Code");
		document.Forders.postcode.focus();
		return false;
		}
	var hp = document.Forders.homeP.value;
	if(isBlank(hp) || isNum(hp) || hp.length !=10 || hp =="Required")
		{
		alert("Please enter a valid Home phone number");
		document.Forders.homeP.focus();
		return false;
		}
	var wp = document.Forders.workP.value;
	if(isNum(wp) || wp.length !=10)
		{
		alert("Please enter a valid Work phone number");
		document.Forders.workP.focus();
		return false;
		}
	var fp = document.Forders.faxP.value;
	if(isNum(fp) || fp.length !=10)
		{
		alert("Please enter a valid fax number");
		document.Forders.faxP.focus();
		return false;
		}
}	
function isNum()// number validation is number
{
	if(isNaN(document.Forders.postcode.value)) 
   		{ 
       	 return false; 
  		 }
   if(isNaN(document.Forders.homeP.value)) 
   		{ 
   		return false; 
   		}
	if(isNaN(document.Forders.workP.value))
		{
		return false;
		}
	if(isNaN(document.Forders.faxP.value))
		{
		return false;
		}
}
function isBlank()//number validation isnt blank
{
	if (document.Forders.postcode.value.length == 0 )
		{
		return false;
		}
	if (document.Forders.homeP.value.length == 0 )
		{
		return false;
		}
}




function valEmail()//Email validation 
{
	var x=document.forms["Forders"]["eMail"].value
	var at=x.indexOf("@");
	var dot=x.lastIndexOf(".");
	if (at<1 || dot<at+2 || dot+2>=x.length)
		{
		alert("Not a valid e-mail address");
		return false;
		}

}
function radioVal()// radio buttons
{
	if (document.Forders.Group1[1].checked)
	{
		if (document.Forders.dStreet.value.length ==0 ||document.Forders.dStreet.value == "Street" ) 
			{
			alert ( "Please enter your delivery address" );
			return false;
			}
		if (document.Forders.dSuburb.value.length ==0 ||document.Forders.dSuburb.value == "Suburb or Town" ) 
			{
			alert ( "Please enter your delivery suburb or town" );
			return false;
			}
		 if(document.Forders.Dpostcode.value.length != 4 || document.Forders.Dpostcode.value =="XXXX" || isNaN(document.Forders.Dpostcode.value))
			 {
			 alert("enter valid delivery postcode");
			 return false; 
			 }
	}
}

function delDrops()
{
	if (document.Forders.Group1[1].checked)
	{
		if(document.Forders.dState.value == "111")
			{
			alert("Please select a state for delivery");
			return false;
			}
		if(document.Forders.Dday.value == "111")
			{
			alert("Please select a date for delivery");
			return false;
			}
		if(document.Forders.Dmonth.value == "111")
			{
			alert("Please select a month for delivery");
			return false;
			}
		if(document.Forders.Dyear.value == "111")
			{
			alert("Please select a year for delivery");
			return false;
			}
	 }
}
function checkText()// check box
{
	if (document.Forders.card.checked)
        {
		if (document.Forders.message.value.length ==0 ||document.Forders.message.value == "Enter your personal message here")
		{
		alert ( "Please enter your personal message in the text area" );
        return false;
		}
        
        }
}

//not valiadation code wax on wax off

function clearIt(field)
{
	if(field.defaultValue==field.value)
	{
		field.value="";
	}
	else if(field.value=="")
	{
		field.value=field.defaultValue;
	}
}


The clearFields function is easiest to make improvements too:


function clearIt() {
    if (this.defaultValue == this.value) {
        this.value = "";
    } else if (this.value == "") {
        this.value = this.defaultValue;
    }
}
function initClearFields(form, fieldNames) {
    var i,
        field;
    for (i = 0; i < fieldNames.length; i += 1) {
        field = form.elements[fieldNames[i]];
        field.onfocus = field.onblur = clearIt;
    }
}

Here’s how you might set it up:


var form = document.getElementById('order');
initClearFields(form, [
    'fName', 'address', 'suburb', 'postcode',
    'homeP', 'workP', 'faxP', 
    'dStreet', 'dSuburb', 'Dpostcode', 'message'
]);

The validation has so many different ways that it can be used, or could be expanded on, so here is one very different variation, which uses some techniques learned from other validator scripts.

It has nowhere near enough documentation, due to it being 4:30 AM, so make of it what you will.


var validator = {
    fieldRules: {},
    rules: {
        required: function (field, required) {
            var dependedField = {};
            
            if (required === false) {
                // no need to validate if it's not required
                // for example:
                //     firstname: {required: false}
                return true;
            } else if (document.getElementsByName(required).length > 0) {
                // Validate field only if the depended field is also valid
                // for example:
                //     period: {required: 'subscription'}
                dependedField = document.getElementsByName(required)[0];
                if (!arguments.callee.call(this, dependedField)) {
                    // if the required field is not found, no need to validate.
                    return true;
                }
            }
            if (field.nodeName === 'INPUT') {
                if (field.type === 'checkbox') {
                    return field.checked;
                } else {
                    return (field.value.length > 0 && field.value !== field.defaultValue);
                }
            }
            if (field.nodeName === 'TEXTAREA') {
                return (field.value.length > 0 && field.value !== field.defaultValue);
            }
            if (field.nodeName === 'SELECT') {
                return (field.selectIndex > 0);
            }
        },
        integer: function (field, args) {
            // validate integer values
            // accepted args include min, max, size
            // for example:
            //     postcode: {required: true, integer: {size: 4}}
            // or
            //     homePhone: {required: true, integer: {min: 8, max: 12}}
            var value = field.value,
                isInteger = (Number(value) === parseInt(value, 10)),
                min = args.min || 0,
                max = args.max || Number.MAX_VALUE;
            if (args.size) {
                min = max = args.size;
            }
            
        },
        email: function (field) {
            // validate emails, that could be [email]a@b.cd[/email] or [email]joe.bloggs@example.com[/email]
            var str = field.value,
                at = str.indexOf("@"),
                dot = str.lastIndexOf(".");
            return (at > 0 && dot - at > 1 && str.length - dot > 2);
        }
    },
    field: function (field, data) {
        var isValid = true,
            rule = data.rules,
            args = data.args,
            message = data.message,
            typeOfRule = '',
            thisRule;

        typeOfRule = typeof rule;
        switch (typeOfRule) {
        case 'function':
           if (field && rule(field, args) === false) {
                alert(message);
                isValid = false;
            }
            break;
        case 'string':
            rule = this.rules[rule];
            if (typeof rule === 'function') {
                isValid = arguments.callee.call(this, field, {
                    rules: rule,
                    args: args,
                    message: message
                });
            }
            break;
        case 'object':
            // loop through multiple validation functions
            for (thisRule in rule) if (rule.hasOwnProperty(thisRule)) {
                isValid = arguments.callee.call(this, field, {
                    rules: thisRule,
                    args: rule[thisRule],
                    message: message
                });
            }
        default:
            // do nothing
        }
            
        return isValid;
    },
    validate: function (form) {
        var isValid = true,
            field = '',
            requirements = {},
            message = '';
        
        for (field in this.fieldRules) if (this.fieldRules.hasOwnProperty(field)) {
            requirements = this.fieldRules[field];
            if (this.field(form.elements[field], requirements) === false) {
                isValid = false;
            }
        }
        if (this.fieldRules.debug) {
            // to make debugging easier, prevent the form from submitting
            // for example:
            //     debug: true
            return false;
        }
        return isValid;
    },
    init: function (fieldRules) {
        this.fieldRules = fieldRules;
    }
};

Here’s what you would do to set it up:


var form = document.getElementById('order');
form.onsubmit = function () {
    return validator.validate(this);
}
validator.init({
    fName: { rules: 'required', message: 'Please enter your name'},
    address: {rules: 'required', message: 'Please enter your address'},
    suburb: {rules: 'required', message: 'Please enter your suburb'},
    postcode: {rules: {'required': true, 'integer': {size: 4}}, message: 'Please enter a valid Post Code'},
    aState: {rules: 'required', message: 'Please select a State'},
    homeP: {rules: {'required': true, 'integer': {size: 10}}, message: 'Please enter a valid Home phone number'},
    workP: {rules: {'integer': {size: 10}}, message: 'Please enter a valid Work phone number'},
    faxP: {rules: {'integer': {size: 10}}, message: 'Please enter a valid fax number'},
    eMail: {rules: 'email', message: 'Not a valid email address'},
    dStreet: {rules: {'required': 'Group1'}, message: 'Please enter your delivery address'},
    dSuburb: {rules: {'required': 'Group1'}, message: 'Please enter your suburb or town'},
    Dpostcode: {rules: {'required': 'Group1'}, message: 'Please enter your delivery postcode'},
    dState: {rules: {'required': 'Group1'}, message: 'Please select a state for delivery'},
    Dday: {rules: {'required': 'Group1'}, message: 'Please select a day for delivery'},
    Dmonth: {rules: {'required': 'Group1'}, message: 'Please select a month for delivery'},
    Dyear: {rules: {'required': 'Group1'}, message: 'Please select a year for delivery'},
    cCard: {rules: 'required', message: 'Please choose your card type'},
    eMonth: {rules: 'required', message: 'Please select card expiry month'},
    eYear: {rules: 'required', message: 'Please select card expiry year'},
    basket: {rules: 'required', message: 'Please make a basket choice'},
    quan: {rules: 'required', message: 'You have chosen 1 basket'},
    message: {rules: {'required': 'card'}, message: 'Please enter your personal message in the text area'}
});

[QUOTE=minusten;4851389]here is a code i recently did

I would be very interested to see possible improvements!:)[/quote]

Here are some alternative improvements, which I’ll take step by step through the process of how they are achieved.

It’s easy to tell that there is a lot of duplication throughout your script.
Let’s remove some of that duplication, so that it’ll be easier to see where the improvements can occur.

Before I get started, I’m goin to run the code through the Online JavaScript Beautifier, so that indenting and other presentational aspects are cleaned up. That will help to make it easier to interpret what the code is doing.

Default Values

The first thing to deal with are where the field value is being checked for a certain string. Doing that forces you to update multiple things whenever you want to update the text. That causes you to not do such updates, which can lead to bad things.

Web pages automatically provide the default text in a property called defaultValue, so we can instead check that the field value equals the fields defaultValue.

Before:

if (document.Forders.fName.value.length == 0 ||
    document.Forders.fName.value == [color="red"]"Please enter your name"[/color]) {

After:

if (document.Forders.fName.value.length == 0 ||
    document.Forders.fName.value == [color="blue"]document.Forders.fName.defaultValue[/color]) {

Similarly with select lists, we can check if the selectedIndex is 0, without needing to know the value that’s there. Whenever the selectedIndex is 0, that refers to the first one that’s selected at the top of the list.

Before:

if (document.Forders.aState.value [COLOR="Red"]== "000"[/COLOR]) {

After:

if (document.Forders.aState.selectedIndex [COLOR="blue"]=== 0[/COLOR]) {

Those updates to remove the default field value strings should be done throughout all of your code.

Passing by Reference

The next form of duplication involves the field names.
In each function, the full field name is referred to over and over again. Take a look at We should assign that common field name to a variable, so that we can use that instead:

Look at for example the nBlank() function:


function nBlank() {
    if ([color="blue"]document.Forders.fName[/color].value.length ==0 ||
        [color="blue"]document.Forders.fName[/color].value == [color="blue"]document.Forders.fName[/color].defaultValue) {
        alert("Please enter your name");
        return false;
    }
    if ([color="blue"]document.Forders.address[/color].value.length == 0 ||
        [color="blue"]document.Forders.address[/color].value == [color="blue"]document.Forders.address[/color].defaultValue) {
        alert("Please enter your address");
        return false;
    }
    if ([color="blue"]document.Forders.suburb[/color].value.length == 0 ||
        [color="blue"]document.Forders.suburb[/color].value == [color="blue"]document.Forders.suburb[/color].defaultValue) {
        alert("Please enter your suburb");
        return false;
    }
}

When we assign those names to a variable called field, the code becomes much more consistent, making it easier for us to improve on.


function nBlank() {
    [color="green"]var field;[/color]
    [color="green"]field = document.Forders.fName;[/color]
    if ([color="green"]field[/color].value.length ==0 ||
        [color="green"]field[/color].value == [color="green"]field[/color].defaultValue) {
        alert("Please enter your name");
        return false;
    }
    [color="green"]field = document.Forders.address;[/color]
    if ([color="green"]field[/color].value.length == 0 ||
        [color="green"]field[/color].value == [color="green"]field[/color].defaultValue) {
        alert("Please enter your address");
        return false;
    }
    [color="green"]field = document.Forders.suburb;[/color]
    if ([color="green"]field[/color].value.length == 0 ||
        [color="green"]field[/color].value == [color="green"]field[/color].defaultValue) {
        alert("Please enter your suburb");
        return false;
    }
}

The above changes have helped us to pinpoint large amounts of duplication within the function, and have also made it easier to remove that duplication.

So what can we do about that? If the field variable is passed in to the form, along with the error message, then you won’t need those three nearly identical pieces of code in there.


function nBlank([color="green"]field, error[/color]) {
    if (field.value.length == 0 || field.value == field.defaultValue) {
        alert([color="green"]error[/color]);
        return false;
    }
}

And when we call nBlank() from the validateForders() function, we can also replace document.Forders with just form


function validateForders(form) {
    ...
    if (nBlank([color="blue"]form.fName, "Please enter your name"[/color]) === false) {
        isValid = false;
    }
    if (nBlank([color="blue"]form.address, "Please enter your address"[/color]) === false) {
        isValid = false;
    }
    if (nBlank([color="blue"]form.suburb, "Please enter your suburb"[/color]) === false) {
        isValid = false;
    }
    ...
}

The good news is that the massive amount of duplication in the nBlank() function is now solved.
The not-so-good news is that now that we’ve removed the duplication from the nBlank() function, there is now duplication in the validateForders() function.

Arrays to the Rescue

Fortunately for us, arrays are made to handle just such an issue. That, combined with associative lists, means that we can solve all our problems.

All we need to do is to put fName, address and suburb in an array, along with their error message. We can then pass that info to a function that will loop through them, and pass them to the nBlank() function for us.

This is also a good time to decide how to group the validation info. If it were books on a shelf, you could group them by size, or by title, or by color, or by category.

With this validation info, two of the common groupings are:

[list][]by type of validation, where all appropriate fields are validated at the same time for being blank
[
]or by type of field, where each field has several different types of validation requirements that they need to pass[/list]

I’ll follow on from here using the first type where groups of fields are validated for being blank, or for being a number, and so on. While there can be some duplication of error messages with this, it’s closer to how you’ve coded your validation script so it should be easier for you to follow.

The field and error message can be put together in an array, where each item of the array handles a different field/error message. Each array item could store the field and message as values of another nested array, but that then goes against the idea that array items should be similar to each other.
So instead, an associative array can be used to store these different pieces of information instead.

We can also make good use of the form variable that is in the validateForders function too:


var notBlanks = [
    {field: form.fName, error: "Please enter your name"},
    {field: form.address, error: "Please enter your address"},
    {field: form.suburb, error: "Please enter your suburb"}
];

We can now loop through each of those items, and assume that things are valid until the nBlank() function tells us that things aren’t.


function validateNotBlanks(fieldData) {
    var field,
        error,
        i;
    for (i = 0; i < fieldData.length; i += 1) {
        field = fieldData[i].field;
        error = fieldData[i].error;
        if (nBlank(field, error) === false) {
            return false;
        }
    }
}

That means that the validateForders() function just needs this code for validating the nBlank() fields:


var notBlanks = [
    {field: form.fName, error: "Please enter your name"},
    {field: form.address, error: "Please enter your address"},
    {field: form.suburb, error: "Please enter your suburb"}
];
if (validateNotBlanks(notBlanks) === false) {
    isValid = false;
}

There is more than just notBlanks to validate though, so it will be useful if we can use the same looping function to validate the other ones. We can do that by passing a reference of nBlank to a validate function, so that the field and the error message can then be passed along to nBlank.


function [color="green"]validate[/color](fieldsToCheck[color="green"], func[/color]) {
    var field,
        error,
        i;
    for (i = 0; i < fieldsToCheck.length; i += 1) {
        field = fieldData[i].field;
        error = fieldData[i].error;
        if ([color="green"]func[/color](field, error) === false) {
            return false;
        }
    }
}

Now we can pass to the validateForders() function a reference to the nBlank function:


var notBlanks = [
    {field: form.fName, error: "Please enter your name"},
    {field: form.address, error: "Please enter your address"},
    {field: form.suburb, error: "Please enter your suburb"}
];
if ([color="green"]validate[/color](notBlanks[color="green"], nBlank[/color]) === false) {
    isValid = false;
}

That code is now easy to understand, and it’s easy to keep it updated too.

Now that we have the above base on which to build upon, we can start to look at the other functions.

Checking Default Labels

Performing similar updates to the cDefault() function gives us:


function cDefault(field, error) {
    if (field.selectedIndex === 0) {
        alert(error);
        return false;
    }
}

There was one set of validation in there that was commented out, and the good news is that you can still keep that in the array list too, if you still want it to be available, but not yet active:


var notDefaultDropdowns = [
    {field: form.aState, error: "Please select a State"},
    {field: form.cCard, error: "Please choose your card type"},
    {field: form.emonth, error: "Please select card expiry month"},
    {field: form.eyear, error: "Please select card expiry year"},
    {field: form.basket, error: "Please make a basket choice"} /*,
    {field: form.quan, error: "You have chosen 1 basket"}*/
];
if (validate(notDefaultDropdowns, cDefault) === false) {
    isValid = false;
}

That was pretty much the same as for the notBlanks() function, which is a nice and useful thing.

Validating Numbers

The numVal() function introduces an interesting issue. Some fields of the code within it are doing more checks than with other fields.


var field = document.Forders.postcode;
if ([color="red"]isBlank(field.value) || [/color]isNum(field.value) || field.value.length != 4) {
 ...
var field = document.Forders.homeP;
if ([color="red"]isBlank(field.value) || [/color]isNum(field.value) || field.value.length != 10[color="red"] ||
    field.value == field.defaultValue[/color]) {
 ...
var field = document.Forders.workP;
if (isNum(field.value) || field.value.length != 10) {

All of those red bits are already handled by the nBlank() function, so we can add them to the notBlanks check validation.


var notBlanks = [
    {field: form.fName, error: "Please enter your name"},
    {field: form.address, error: "Please enter your address"},
    {field: form.suburb, error: "Please enter your suburb"},
    [color="blue"]{field: form.postcode, error: "Please enter a valid Post Code"},
    {field: form.homeP, error: "Please enter a valid Home phone number"}[/color]
];

That now leaves the numVal function more consistent, and capable of being condensed with less repetition.
Some of the lengths that are checked though are different in size. We can deal with that by passing to the function the size that we want. How do we pass in the size?

Additional Data Required

When validating your page, you will sometimes find that you need to provide more info than just the type of validation and an error message. You might have min/max/size/range needs to take care of, or you may need to pass on other instructions relating to the form.

To help deal with all of these different needs, we can have a third generic argument called data. That way the validate() function can pass any extra needed info to the function that needs it.


function validate(fieldsToCheck, func) {
    var field,
        [color="green"]data,[/color]
        error,
        i;
    for (i = 0; i < fieldsToCheck.length; i += 1) {
        field = fieldData[i].field;
        [color="green"]data = fieldData[i].data;[/color]
        error = fieldData[i].error;
        if (func(field, error[color="green"], data[/color]) === false) {
            return false;
        }
    }
}

function numVal(field, error[color="green"], data[/color]) {
    if (isNum(field.value) || field.value.length != [color="green"]data.size[/color]) {
        alert(error);
        field.focus();
        return false;
    }
}

The numeric values can now be validated, with their size requirements passed along in the data section.


var numericValues = [
    {field: form.postcode, [color="green"]data: {size: 4},[/color] error: "Please enter a valid Post Code"},
    {field: form.homeP, [color="green"]data: {size: 10},[/color] error: "Please enter a valid Home phone number"},
    {field: form.workP, [color="green"]data: {size: 10},[/color] error: "Please enter a valid Work phone number"},
    {field: form.faxP, [color="green"]data: {size: 10},[/color] error: "Please enter a valid fax number"}
];
if (validate(numericValues, numVal) === false) {
    isValid = false;
}

Currently the numVal() function only handles the size property, but it wouldn’t be difficult at all to extend that to handle things like min, max and range too.

Setting the Focus

You might have noticed too that the numVal() function focuses the field on an error, while the nBlank() function does not. How can we have it check the postcode and homeP, and have those fields still gain the focus?

We can add another piece of data to say whether we want the field to be focussed, or not. Since more than just the nBlank() function might want to focus the field, we should check for that focus part from the validate function itself:


function validate(fieldsToCheck, func) {
    ...
        if (func(field, error, data) === false) {
            [color="green"]if (data.focus === true) {
                field.focus();
            }[/color]
            return false;
        }
    ...
}

Now we can easily tell the postcode and homeP fields to set their focus when the error occurs, as well as all of the numeric fields too.


var notBlanks = [
    {field: form.fName, error: "Please enter your name"},
    {field: form.address, error: "Please enter your address"},
    {field: form.suburb, error: "Please enter your suburb"},
    {field: form.postcode, [color="green"]data: {focus: true},[/color] error: "Please enter a valid Post Code"},
    {field: form.homeP, [color="green"]data: {focus: true},[/color] error: "Please enter a valid Home phone number"}
];
var numericValues = [
    {field: form.postcode, data: {size: 4[color="green"], focus: true[/color]}, error: "Please enter a valid Post Code"},
    {field: form.homeP, data: {size: 10[color="green"], focus: true[/color]}, error: "Please enter a valid Home phone number"},
    {field: form.workP, data: {size: 10[color="green"], focus: true[/color]}, error: "Please enter a valid Work phone number"},
    {field: form.faxP, data: {size: 10[color="green"], focus: true[/color]}, error: "Please enter a valid fax number"}
];

Of course, it’s entirely up to you to decide if you always want the error field to gain focus. If that’s the case, you don’t need the data focus part at all. You can just remove that if statement from the validate function so that the field will always focus after showing the error message.

Regardless of that, after the numVal() function has the focus parts removed from it, that now leaves it looking remarkably similar to the cBlank() and nDefault() functions:


function nBlank(field, error) {
    if (field.value.length == 0 || field.value == field.defaultValue) {
        alert(error);
        return false;
    }
}
function cDefault(field, error) {
    if (field.selectedIndex === 0) {
        alert(error);
        return false;
    }
}
function numVal(field, error, data) {
    if (isNum(field.value) || field.value.length != data.size) {
        alert(error);
        return false;
    }
}

Error Messages

Those three functions help us to see that the error alert is a common thing that can be brought up to the validate() function.

If we bring the error alerting to the validate function, those three functions will only need to return true or false, so they can be reduced to the following:


function nBlank(field) {
    return (field.value.length == 0 || field.value == field.defaultValue);
}
function cDefault(field) {
    return (field.selectedIndex === 0);
}
function numVal(field, data) {
    return (isNum(field.value) || field.value.length != data.size);
}

With the error alert now occurring in the validate() function:


function validate(fieldsToCheck, func) {
    ...
    if (func(field, error) === false) {
        [color="green"]alert(error);[/color]
        ...
    }
    ...
}

So, numVal() is now tidied up, and since the isBlank() function was only used by numVal(), the isBlank() function can be removed completely.

That leaves four different functions to go: valEmail(), radioVal(), delDrops() and checkText()

Validating Email

With the valEmail() you can just pass the field in there, and you’re nearly done:

I won’t get in to all of the different ways to validate emails. To validate them properly can get complex and scary. There is however the issue of those x’s.


function valEmail([color="green"]field[/color]) {
    var [color="red"]x[/color] = [color="green"]field[/color].value
    var at = [color="red"]x[/color].indexOf("@");
    var dot = [color="red"]x[/color].lastIndexOf(".");
    return (at < 1 || dot < at + 2 || dot + 2 >= [color="red"]x[/color].length);
}

That x variable needs to be improved since it doesn’t say much on its own. What is x. Does it mark the spot, is it a fashion accessory, or is it a random variable name because nothing else seemed appropriate.

Let’s have the variable name give us at least some idea or what it contains, by calling it email.


function valEmail(field) {
    var [color="green"]email[/color] = field.value
    var at = [color="green"]email[/color].indexOf("@");
    var dot = [color="green"]email[/color].lastIndexOf(".");
    return (at < 1 || dot < at + 2 || dot + 2 >= [color="green"]email[/color].length);
}

The final thing that I would tidy up in the valEmail() is the comparison.
Instead of checking for three different fail conditions:
(a || b || c)

We can instead check that three pass conditions aren’t met:
!(!a && !b && !c)

Does that sound crazy? Bear with me.

Inverting a less than comparison just means checking for greater than or equal. We can also get rid of that invert at the start too, by checking if the whole lot equals to false. That also causes it to use the same structure as all of our other validation functions. Happy joy!

So after inverting everything we have:

(at [color="blue"]>=[/color] 1 [color="blue"]&&[/color] dot [color="blue"]>=[/color] at + 2 [color="blue"]&&[/color] dot + 2 [color="blue"]<[/color] str.length) [color="blue"]=== false[/color]

If we now move the variables to the left of the comparison, and the numbers to the right, we end up with this:

(at >= 1 && dot [color="green"]- at[/color] >= 2 && [color="green"]str.length -[/color] dot [color="green"]>= 2[/color]) === false

That can be much easier to understand. When it’s easier to tell at a glance what it being checked for, it makes it that much harder for bugs to remain in the code.

You could also replace it with a regular expression, such as:

/^.{1,}@.{1,}\\..{2,}$/.exec(email)

which does exactly the same job as the comparison, but easily understanding what a regular expression does can be something that even experts have trouble with.

With the above changes in place, the valEmail() function ends up being:


function valEmail(field) {
    var email = field.value
    var at = email.indexOf("@");
    var dot = email.lastIndexOf(".");
    return ((at >= 1 && dot - at >= 2 && email.length - dot >= 2) === false);
}

And it’s called with:


var emailValues = [
    {field: form.eMail, error: "Not a valid e-mail address"}
];
if (validate(emailValues, valEmail) === false) {
    isValid = false;
}

Depending on Dependencies

The radioVal() function has a lot of duplication there too, that we can remove:

What we are starting with:

function radioVal() {
    if (document.Forders.Group1[1].checked) {
        var field = document.Forders.dStreet;
        if (field.value.length == 0 || field.value == field.defaultValue) {
            alert("Please enter your delivery address");
            return false;
        }
        var field = document.Forders.dSuburb;
        if (field.value.length == 0 || field.value == field.defaultValue) {
            alert("Please enter your delivery suburb or town");
            return false;
        }
        var field = document.Forders.Dpostcode;
        if (field.value.length != 4 || field.value == field.defaultValue || isNaN(field.value)) {
            alert("enter valid delivery postcode");
            return false;
        }
    }
}

I think that the radioVal() function has moved far away from validating radio buttons. Instead of validating radio buttons, it’s just ensuring that the normal validation of some fields depends on that radio button being selected.

That dependency requirement can be added as data to the field info, which can then be used by our standard validation routine.


var notBlanks = [
    ...
    {field: form.dStreet, [color="green"]data: {dependsOn: form.Group1[1]},[/color] error: "Please enter your delivery address"},
    {field: form.dSuburb, [color="green"]data: {dependsOn: form.Group1[1]},[/color] error: "Please enter your delivery suburb or town"},
    {field: form.Dpostcode, [color="green"]data: {dependsOn: form.Group1[1]},[/color] error: "Please enter valid delivery postcode"}
];
var numericValues = [
    ...
    {field: form.Dpostcode, [color="green"]data: {dependsOn: form.Group1[1], size: 4},[/color] error: "Please enter a valid delivery postcode"}
];

Now we just need the validate function to look for that dependsOn condition, and only do the validation when that dependsOn field is active. To help keep things nice and simple, we can hand off that task to a separate function:


function validate(fieldsToCheck, func) {
    ...
    if ([color="green"]checkDependency(data.dependsOn) &&[/color] func(field, error) === false) {
        alert(error);
        if (data.focus === true) {
            field.focus();
        }
        return false;
    }
    ...
}

So if the checkDependency() function returns true, the validate() function will carry on and validate the field. If checkDependency() returns false, the validate() function just skips that field and moves on with the next.

As you can see here, the things that it checks for are very basic, but quite useful.


function checkDependency(field) {
    if (field.type === 'checkbox') {
        return field.checked;
    }
    if (field.type === 'radio') {
        return field.selected;
    }
    if (field.type === 'text') {
        return (field.value.length > 0 && field.value !== field.defaultValue);
    }
}

Now the radioVal() function can be deleted.

The delDrops() function has a similar situation, so we can handle it now with only these additions:


var notDefaultDropdowns = [
    ...
    {field: form.dState, [color="green"]data: {dependsOn: form.Group1[1]},[/color] error: "Please select a state for delivery"},
    {field: form.Dday, [color="green"]data: {dependsOn: form.Group1[1]},[/color] error: "Please select a date for delivery"},
    {field: form.Dmonth, [color="green"]data: {dependsOn: form.Group1[1]},[/color] error: "Please select a month for delivery"},
    {field: form.Dyear, [color="green"]data: {dependsOn: form.Group1[1]},[/color] error: "Please select a year for delivery"}
];

And now the delDrops() function can also be removed.

That brings us to the last function: checkText(), to which we can apply the same dependency solution.


var notBlanks = [
    ...
    {field: form.message, [color="green"]data: {dependsOn: form.card},[/color] error: "Please enter your personal message in the text area"}
];

Bringing things Together

So now the validateForders() function contains two main things. It contains a series of arrays at the start:


var notBlanks = [
    ...
];
var numericValues = [
    ...
];
var notDefaultDropdowns = [
    ...
];
var emailValues = [
    ...
];

And a series of validating calls below them:



if (validate(notBlanks, nBlank) === false) {
    isValid = false;
}
if (validate(numericValues, numVal) === false) {
    isValid = false;
}
if (validate(notDefaultDropdowns, cDefault) === false) {
    isValid = false;
}
if (validate(emailValues, valEmail) === false) {
    isValid = false;
}

Wouldn’t it be great if we didn’t need those duplicate validation calls?

What we can do about that is to have a final array called fieldsToValidate, in which we associate each array with the function to validate it with.


groupsToValidate = [
    {group: notBlanks, validator: nBlank},
    {group: numericValues, validator: numVal},
    {group: notDefaultDropdowns, validator: cDefault},
    {group: emailValues, validator: valEmail},
    
];

This is also a good time to rename the functions to other names, if you want to make them consistent and expressive of what they do.

Now we can just loop through each of those groups, making the same call to the validate function.


for (i = 0; i < groupsToValidate.length; i += 1) {
    if (validate(groupsToValidate.group, groupsToValidate.validator) === false) {
        isValid = false;
    }
}

That will show an error message for each type of validation.

If you only want to show one error message and then stop, that can be easily done too, by breaking out of the loop when an error is detected.


for (i = 0; i < groupsToValidate.length; i += 1) {
    if (validate(groupsToValidate.group, groupsToValidate.validator) === false) {
        isValid = false;
        [color="green"]break;[/color]
    }
}

The resulting code from all of these updates is:


function validateForders(form) {
    var isValid = true,
        notBlanks, numericValues, notDefaultDropdowns, emailValues,
        groupsToValidate,
        i;
    
    notBlanks = [
        {field: form.fName, error: "Please enter your name"},
        {field: form.address, error: "Please enter your address"},
        {field: form.suburb, error: "Please enter your suburb"},
        {field: form.postcode, data: {focus: true}, error: "Please enter a valid Post Code"},
        {field: form.homeP, data: {focus: true}, error: "Please enter a valid Home phone number"},
        {field: form.dStreet, data: {dependsOn: form.Group1[1]}, error: "Please enter your delivery address"},
        {field: form.dSuburb, data: {dependsOn: form.Group1[1]}, error: "Please enter your delivery suburb or town"},
        {field: form.Dpostcode, data: {dependsOn: form.Group1[1]}, error: "Please enter valid delivery postcode"},
        {field: form.message, data: {dependsOn: form.card}, error: "Please enter your personal message in the text area"}
    ];
    numericValues = [
        {field: form.postcode, data: {size: 4, focus: true}, error: "Please enter a valid Post Code"},
        {field: form.homeP, data: {size: 10, focus: true}, error: "Please enter a valid Home phone number"},
        {field: form.workP, data: {size: 10, focus: true}, error: "Please enter a valid Work phone number"},
        {field: form.faxP, data: {size: 10, focus: true}, error: "Please enter a valid fax number"},
        {field: form.Dpostcode, data: {dependsOn: form.Group1[1], size: 4}, error: "Please enter a valid delivery postcode"}
    ];
    notDefaultDropdowns = [
        {field: form.aState, error: "Please select a State"},
        {field: form.cCard, error: "Please choose your card type"},
        {field: form.emonth, error: "Please select card expiry month"},
        {field: form.eyear, error: "Please select card expiry year"},
        {field: form.basket, error: "Please make a basket choice"},
        /*{field: form.quan, error: "You have chosen 1 basket"},*/
        {field: form.dState, data: {dependsOn: form.Group1[1]}, error: "Please select a state for delivery"},
        {field: form.Dday, data: {dependsOn: form.Group1[1]}, error: "Please select a date for delivery"},
        {field: form.Dmonth, data: {dependsOn: form.Group1[1]}, error: "Please select a month for delivery"},
        {field: form.Dyear, data: {dependsOn: form.Group1[1]}, error: "Please select a year for delivery"}
    ];
    emailValues = [
        {field: form.eMail, error: "Not a valid e-mail address"}
    ];
    groupsToValidate = [
        {group: notBlanks, validator: nBlank},
        {group: numericValues, validator: numVal},
        {group: notDefaultDropdowns, validator: cDefault},
        {group: emailValues, validator: valEmail},
        
    ];
    
    for (i = 0; i < groupsToValidate.length; i += 1) {
        if (validate(groupsToValidate.group, groupsToValidate.validator) === false) {
            valid = false;
            break;
        }
    }
    
    return isValid;
}
function validate(fieldsToCheck, func) {
    var field,
        data,
        error,
        i;
    for (i = 0; i < fieldsToCheck.length; i += 1) {
        field = fieldData[i].field;
        data = fieldData[i].data;
        error = fieldData[i].error;
        if (checkDependency(data.dependsOn) && func(field, error, data) === false) {
            alert(error);
            if (data.focus === true) {
                field.focus();
            }
            return false;
        }
    }
}
function checkDependency(field) {
    if (field.type === 'checkbox') {
        return field.checked;
    }
    if (field.type === 'radio') {
        return field.selected;
    }
    if (field.type === 'text') {
        return (field.value.length > 0 && field.value !== field.defaultValue);
    }
}
function nBlank(field) {
    return (field.value.length == 0 || field.value == field.defaultValue);
}
function cDefault(field) {
    return (field.selectedIndex === 0);
}
function numVal(field, data) {
    return (isNum(field.value) || field.value.length != data.size);
}
function valEmail(field) {
    var email = field.value
    var at = email.indexOf("@");
    var dot = email.lastIndexOf(".");
    return ((at >= 1 && dot - at >= 2 && str.length - dot >= 2) === false);
}