Can people understand my form validation code?

I've just finished creating this form validation but I want to make it public for beginner 'contact us forms'.

I was wondering if I can have some peoples' input on if they can understand/read my JavaScript. Although I know my JS works, it's better to be safe than to have 20 people asking what this does. It's just input feedback.

What I'm trying to achieve in my code is making fields required with JavaScript.

The following fields are REQUIRED:

• First Name

• Last Name

• Email

• Phone

Most of the functions such as alphaNumeric only allows numbers and onlyAlphameric only allows letters.

E.g for each function or what not, add a comment saying what it does.

For example, adding a comment:

This function does this and triggers that and so on.

JavaScript:

function elem(id) {
    return document.getElementById(id);
};

window.onload = function () {
    document.querySelector("#RadioGroup1_0").click();

    var form = document.getElementById('form');
    form.onsubmit = function (e) {
        var rules = [
        ['first-name', elem('first-name').value.length > 0],
        ['last-name', elem('last-name').value.length > 0],
        ['email', elem('email').value.length > 0 && /^([a-zA-Z0-9_\.\-])+\@(([a-zA-Z0-9\-])+\.)+([a-zA-Z0-9]{2,4})+$/.test(elem('email').value)],
        ['phone', elem('phone').value.length > 7 && elem('phone').value.length  64 && k  96 && k  64 && charCode  96 && charCode = 48 && keyCode <= 57) || specialKeys.indexOf(keyCode) != -1);
    //document.getElementById("phone").style.display = ret ? "none" : "inline";
    return ret;
}

Can people understand my code?

Its certainly not bad code - but I believe there are some things you can do which will improve the speed with which people like me can grasp what it is doing (and why).

Comments

Some people think well-written code needs no comments. I don't fall into that camp. Looking at your code I see the following:

Total number of comments is 2

Total number of helpful comments is 1

Total number of comments describing goal of sections of code is 0

Languages like Java have JavaDoc, which are comments in the code from which you can automatically produce documentation. If you've ever tried to re-use code that has no documentation you'll know how useful it is. If you've ever had to maintain documentation that is separate from code you'll know what a pain that is. Innaccurate documentation is worse than none.

So I believe it is best to put documentation into code. Ideally in a form from which documentation can be produced.

I like to treat every function/method I write as if it were one I would put into a library and re-use. To me this means it needs comments that document its purpose, interface and any quirks.

Magic Numbers

You have a lot of magic numbers, e.g. 48 and 57. These should be either

• explained in comments

• replaced by constants with meaningful names (e.g. ASCII_0, DIGIT_0 ...)

• replaced by functions

The latter may be best, especially if you (or anyone else) want this code to work with character sets other than ASCII.

Meaningful names

function myFunction()

I feel you may be underestimating the usefulness of meaningful names as an aid to other's understanding. I'd try to find a more helpful name that helps others understand the purpose of the the function and what it returns.

Writing for comprehension

You use the ternary operator. Often this is used for assignment. If you use if for assignment but in an unusual way you may momentarily confuse readers.

Unless you need conciseness, it can sometimes be clearer to use a conventional if-then-else form instead.

Wouldn't

document.all ? k = e.keyCode : k = e.which;

be clearer as

k = document.all ? e.keyCode : e.which;

Because the main purpose seems to be to assign a value to k. The earlier style is one I might use if the main point is to call a function and, as an afterthought, evaluate its's success or failure. I think in this case it moves the reader's focus to the wrong place.

Writing portable code and avoiding browser-sniffing

The above use of the ternary operator can probably be avoided entirely. I suspect you may be interested in jQuery and replace it with

keyPressed = e.which

Which I find easier to read and which can be understood more rapidly.

I would at least factor out browser-dependent and/or repeated stuff into my own functions/methods and/or carefully document in comments what it is doing and why it needs to be done.

Documenting purpose


What I'm trying to achieve in my code is ...

As I mentioned above, I believe it is very useful to have this described in comments in the code itself (others may disagree). From it's inclusion in your question you evidently believe this helps readers understand the following code. Therefore I'd put it into the code as a comment.

StackExchange Code Review Q#49218, answer score: 15