Tell me more ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 0 down vote favorite

I'm currently working on creating an expenses form and am writing a function to validate the entered data.

I allow for 6 expense amounts to be entered and a description on each and onClick of the submit button I am trying to write a for loop that will loop round the amount fields, check if they are numeric and, if they are, then check to ensure a description has been entered.

The form basically looks like:

<form action="##" name="myForm" id="myForm" method="post">
    <input type="text" name="otherExpenseDesc1" id="otherExpenseDesc1">
    <input type="text" name="otherExpenseAmount1" id="otherExpenseAmount1">
    <input type="text" name="otherExpenseDesc2" id="otherExpenseDesc2">
    <input type="text" name="otherExpenseAmount2" id="otherExpenseAmount2">
    <input type="text" name="otherExpenseDesc3" id="otherExpenseDesc3">
    <input type="text" name="otherExpenseAmount3" id="otherExpenseAmount3">
    <input type="text" name="otherExpenseDesc4" id="otherExpenseDesc4">
    <input type="text" name="otherExpenseAmount4" id="otherExpenseAmount4">
    <input type="text" name="otherExpenseDesc5" id="otherExpenseDesc5">
    <input type="text" name="otherExpenseAmount5" id="otherExpenseAmount5">
    <input type="text" name="otherExpenseDesc6" id="otherExpenseDesc6">
    <input type="text" name="otherExpenseAmount6" id="otherExpenseAmount6">
    <input type="submit" name="formSubmitBtn" value="SUBMIT" onClick="checkForm();">
</form>

And the javascript I have at the moment is:

function checkForm() {
    var errMsg = "";
    var otherExpense1 = document.getElementById('otherExpenseAmount1').value;
    var otherExpenseDesc1 = document.getElementById('otherExpenseDesc1').value;
    var otherExpense2 = document.getElementById('otherExpenseAmount2').value;
    var otherExpenseDesc2 = document.getElementById('otherExpenseDesc2').value;
    var otherExpense3 = document.getElementById('otherExpenseAmount3').value;
    var otherExpenseDesc3 = document.getElementById('otherExpenseDesc3').value;
    var otherExpense4 = document.getElementById('otherExpenseAmount4').value;
    var otherExpenseDesc4 = document.getElementById('otherExpenseDesc4').value;
    var otherExpense5 = document.getElementById('otherExpenseAmount5').value;
    var otherExpenseDesc5 = document.getElementById('otherExpenseDesc5').value;
    var otherExpense6 = document.getElementById('otherExpenseAmount6').value;
    var otherExpenseDesc6 = document.getElementById('otherExpenseDesc6').value;

    for (i=1; i<7; i++) {       
        expenseNo = 'otherExpense' + i;
        expenseDescNo = 'otherExpenseDesc' + i;
        if (expenseNo != "") {
            if (isNaN(expenseNo)) {
                alert(expenseNo + ' is not a number');
                errMsg = errMsg + 'The amount must be a number.<br />';
                document.getElementById('otherExpenseAmount' + i).style.border = '2px sold #990000';
            } else {
                alert('otherExpenseAmount' + i + ' is a number');
                document.getElementById('otherExpenseAmount' + i).style.border = '2px sold #990000';
            }
        }
    }

    if (errMsg != "") {
        showDialog('alert', 'OK', '', errMsg, '');
    }
}

When I'm submitting the form with valuesin a couple of the fields it still displays an alert for each otherExpenseAmount item saying that it is not numeric, although it is, and the border style doesn't change.

Any idea where I'm going wrong?

share|improve this question

2 Answers

up vote 1 down vote accepted

You're trying to access a variable with a string. You should change your code to utilise arrays, something like this:

jsFiddle

function checkForm() {
    var errMsg = "";
    var otherExpense = [];

    otherExpense.push(document.getElementById('otherExpenseAmount1').value);
    otherExpense.push(document.getElementById('otherExpenseAmount2').value);
    otherExpense.push(document.getElementById('otherExpenseAmount3').value);
    otherExpense.push(document.getElementById('otherExpenseAmount4').value);
    otherExpense.push(document.getElementById('otherExpenseAmount5').value);
    otherExpense.push(document.getElementById('otherExpenseAmount6').value);

    // 0-based index for arrays
    for (i=0; i<6; i++) { 
        if (otherExpense[i] != "") {
            if (isNaN(otherExpense[i])) {
                alert('otherExpense ' + (i + 1) + ' is not a number (=' + otherExpense[i] + ')');
                errMsg = errMsg + 'The amount must be a number.<br />';
                document.getElementById('otherExpenseAmount' + (i + 1)).style.border = '2px sold #990000';
            } else {
                alert('otherExpenseAmount' + (i + 1) + ' is a number');
                document.getElementById('otherExpenseAmount' + (i + 1)).style.border = '2px sold #990000';
            }
        }
    }

    if (errMsg != "") {
        alert(errMsg);
        //showDialog('alert', 'OK', '', errMsg, '');
    }

    return false;
}
share|improve this answer
Thanks Tyriar. What exactly does the push method do? – user1523361 Mar 28 at 11:07
push(x) adds an element to the end of an array. – Daniel Imms Mar 28 at 11:10
up vote 0 down vote

Your problem is on these lines; you're setting the border to be the same, regardless of the condition (easy mistake to make).

if (isNaN(expenseNo)) {
    alert(expenseNo + ' is not a number');
    errMsg = errMsg + 'The amount must be a number.<br />';
    document.getElementById('otherExpenseAmount' + i).style.border = '2px sold #990000';
// -----------------------------------------------------------------------------^^^^^^
} else {
    alert('otherExpenseAmount' + i + ' is a number');
    document.getElementById('otherExpenseAmount' + i).style.border = '2px sold #990000';
// -----------------------------------------------------------------------------^^^^^^
}

Suggestion

I hate to be that guy.

Have you considered adopting jQuery? It's perfectly designed for tasks such as this. Utilising methods such as filter and simple selectors, it's easy to determine whether all inputs fulfil your requirements. Here's an example:

// select all inputs with a class of "number"
// then filter those inputs, selecting only those where the value is "not a number"
// if there is a length, i.e. more than 0 were found, alert a message

if ($('input.number').filter(function() { return isNaN(this.value); }).length) {
    alert('Please ensure all numbers are valid');   
}

This would obviously require you to make small amendments to your HTML, by adding the number class to the required elements, but it's a small sacrifice to make.

jsFiddle demo

share|improve this answer
The reason for setting the border the same was just to ensure that that particular line of code was being ran but I've just realised I've made a typo which is why that wasn't working. I've used jQuery for other functions on the page but have never really dabbled with it for form validation but now I know how I may well do, thanks. – user1523361 Mar 28 at 11:04

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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