Form validation appears to be too rigid

A Stack Overflow coder said my code is too rigid and inflexible. Would someone critique my form validation code, and give some pointers on being more efficient?

Code Logic:

-
If the field has an error (empty or a RegEx fail) with either on submit or on change, the user will see a suitable message indicating the error and a Unicode arrow pointing to the invalid field.

-
If the the field has passed, the arrow and error message are not displayed.

Link to JsFiddle Code Sample

`




form validation

body, h1, input, div {
font-family: arial;
}
input[type="text"],
input[type="password"] {
font-size: 1.15em;
padding: 0.4em 0.5em;
}
/ Start red arrow error messages below input fields /
.error-text-login{
color: #ea4040;
vertical-align: bottom;
text-align: middle;
width: 315px;
display: none;
}
.error-text-login:before {
content: '\0020 \21E1 \0020';
vertical-align: bottom;
}
.error-text-login {
font-size: 0.75em;
}
.error-text-login:before {
font-size: 1.3em;
}
/ End red arrow error messages below input fields /
#usernameLogin.error input,
#passwordLogin.error input {
background-color: #fff7f7;
border: 1px solid #f68c8c;
border-radius: 2px;
box-shadow: 0 0 6px #ffeded inset;
color: #e10000;
}

#usernameLogin.default input,
#passwordLogin.default input {
background-color: #f7f7f5;
border: 1px solid #c1c1c1;
border-radius: 2px;
box-shadow: 0 0 6px #eeeeee inset;
color: #a3a3a3;
}




Login

Sure. I think one piece of advice I often give to people is to always use readable variable names. It's not your job to compress the variable names for a smaller script download, a minifier can do that. So name things like password instead of pswd, for future maintainability.

You can simplify your 'empty' checking by testing that the field has a length property:

// Too many checks that all do the same thing, hard to read
if (userName == "" || userName == null || lenUserName <= 0) {

// One check, simple and readable
if (!userName.length) {

In fact, you have cached the length in a variable before, so let's use it. Note that it's good practice to put the positive conditions before the negative ones.

if (lenUserName) {
    // everything is OK
} else if (!lenUserName) {
    // validation failed
}

I would also consider renaming this to something a little more explicit, such as usernameLength or userNameLength.

You can save yourself writing return true and return false by returning the boolean value of an expression:

// Too long winded
if (errorQ == same) {
    return true; 
} else {    
    return false;
}

// Can be one line - and you should use strict equality checking!
return errorQ === same;

You should look at caching the result of DOM lookups, for a slight performance benefit, less code download and error prevention (repeating the same code again could lead to typos).

if (pswd == "" || pswd == null || pswd  0) {
    document.getElementById('passwordLogin').className = 'default';
    document.getElementById('pswd-error').innerHTML = "";
    document.getElementById('pswd-error').style.display = "none";
}

// After utilising caching, etc

var passwordLogin = document.getElementById('passwordLogin'),
    passwordError = document.getElementById('pswd-error');

if (lenPswd) {
    passwordLogin.className = 'default';
    passwordError.innerHTML = "";
    passwordError.style.display = "none";
} else if (!lenPswd) {
    passwordLogin.className = 'error';
    passwordError.innerHTML = "Password is required";
    passwordError.style.display = "block";
    errorQ += "password cannot be empty \n";
}

I think you have also repeated yourself by creating an anonymous function for both the onsubmit and onchange handlers with the same contents. You should do something like the following:

var formValidation = function() {
    // your custom code
};

var form = document.getElementById("theForm");

form.onsubmit = formValidation;
form.onchange = formValidation;

Finally, you should look into abstracting your required field checking into a method itself, saving you from having to duplicate that logic for each field that you were to add to the form. Such an implementation could take this form:

var validateField = function(value, element, error, message) {
    if (value.length) {
        element.className = 'default';
        error.innerHTML = "";
        error.style.display = "none";
    } else {
        element.className = 'error';
        error.innerHTML = message;
        error.style.display = "block";
        errorQ += message + ' \n';
    }
};

// Example for the password field
validateField(
    document.forms[0].pswd.value, 
    document.getElementById('passwordLogin'),
    document.getElementById('pswd-error'),
    'Password is required'
);

Hope this helps.

StackExchange Code Review Q#64132, answer score: 3