Javascript form parser

Question

I am trying to make a generic form parser for Javascript. The idea being, a developer could drop the function onto a form and sumbit via AJAX, or do something with a form besides submit it. The result will be in the same format used in a query string.

What I have posted works, so I am basically wondering if this is a good design? What could be improved? Thanks for the help!

function test_submit(id){
    var results=[],form=document.getElementById(id);
    for(var i=0;i<form.elements.length;++i){
        var obj=form.elements[i];
        if(obj.name&&!obj.disabled){
            switch(obj.tagName){
            case 'SELECT':
                for(var j=0;j<obj.length;++j){
                    if(obj.options[j].selected){
                        var value=obj.options[j].value;
                        if(!value)value=obj.options[j].text;
                        results.push(obj.name+'='+escape(value));
                    }
                }
                break;
            case 'INPUT':
            case 'TEXTAREA':
                var type=obj.type,value=obj.value;
                if(type)type=type.toLowerCase();
                if(type&&(type=='radio'||type=='checkbox')){
                    if(obj.checked)results.push(obj.name+'='+(value?escape(value):'on'));
                }
                else results.push(obj.name+'='+escape(value));
                break;
            }
        }
    }
    return results.join('&');
}

Answer 1

I'd change two things. I'd start by declaring your var's in one statement like so:

function test_submit(id){
    var q=[],
        p=$(id).getElementsByTagName('SELECT');

Second I'd use more descriptive variable names than i, p, r, n. If you run this code through a minifier it'll do this for you. When you come back to edit this code in a few months you'll want to know exactly what each variable is for, and using a descriptive name helps with that.

I'd also be leary of using $ in the global namespace as a variable name since a lot of the popular libraries use it and you don't want to have any collisions with those.

Answer 2

In regards to variables i and j they are loop index variables and can be declared in the loop alone instead of at the top. It will clean up the code.

Example

for (var j=0;j<obj.length;++j){...}

Personally I would write it up like this describing p and i which are form elements. In additon there is no need to duplicate code (example would be 3 identical for statments), you can gather all the form elements and concat the arrays together in one. Then by using a switch statement you can minimize the duplicate ifs for each element type and eliminate the need for duplicate for statements. Have a look below. it is much cleaner.

function test_submit(id){  
    var params=[],
        results=[],
        form=document.getElementById(id),
        obj;  

    for(var i = 0, var formElement = form.elements[i]; i < form.elements.length; i++) {  
        if (formElement.name) {
            switch (formElement.tagName) {
                case "SELECT":
                    for(var j=0, var optionItem = formElement.options[j]; j < formElement.length; j++){  
                        if(optionItem.selected){  
                            var value = optionItem.value;  
                            if(!value) value = optionItem.text;  
                            params.push([formElement.name,value]);  
                        }  
                    }  
                    break;
                case "INPUT":
                    var type=formElement.type,
                        value=formElement.value;  
                    if(type) type=type.toLowerCase();  
                    if(type && (type == 'radio' || type == 'checkbox')) {  
                        if(formElement.checked) params.push([formElement.name,value?value:'on']);  
                    }  
                    else params.push([formElement.name,value]);  
                    break;
                case "TEXTAREA":
                    params.push([formElement.name,formElement.value]);  
                    break;
            }
        }
    }
    for (var i in params) results.push(params[i][0]+'='+escape(params[i][1]));  
    return results.join('&');  
}  

Answer 3

Elegant solution overall. Try jQuery as it has some nice features that can let you do some of this in one-liners. Code-on brother.

Answer 4

A few things:

1 I'd like to see it handle password fields separately: you might want to process them in some way, obfuscation, validation, strength testing etc.

2 Personally, I'd avoid using 'form' as a variable name. Just for the sake of my own sanity, if nothing else.

3 What if the form elements have no names?