JavaScript: Optimizing against Redundancy (Loop + Switch + Key/Value)

Question

Is there a better way to do this?

I have a function element.getFormData that builds a parameterized string if called with any element in a form, like a submit button. It's just a simple loop to gather all the input values in the form. What I like about it, is that it also hard returns if it detects a file upload, which you have to do anyway, so that the appropriate alternative method can handle that case. What I don't like about it is the redundancy in the loop. I will show you what I mean.

element.getFormData     = 
function(_R)
{   var vars="";var form=this,i=0,n,o,en=encodeURIComponent;

    while(form){if('FORM'===form.tagName)break;form=form.parentNode}
        form=form.getElementsByTagName('*');

    while(n=form[i++])
    {   switch((n.type||'').toLowerCase())
        {   case'file':
                if(_R)
                    return false;
            case'radio':
            case'checkbox':
                if(n.checked)
                    vars+=  "&" +   en(n.name||n.id)
                        +   "=" +   en(n.value);
            break;
            break;
            case'select-multiple':
                for(var k=0;o=n.options[k++];)
                    if(o.selected)
                        vars+=  "&"     +   en(n.name||n.id)
                            +   "="     +   en(o.value||o.text);
            break;
            default:
                switch(n.tagName)
                {   case'INPUT':
                    case'TEXTAREA':
                        vars+=  "&" +   en(n.name||n.id)
                            +   "=" +   en(n.value);
                    break;
                    break;
                    case'SELECT':
                        vars+=  "&" +   en(n.name||n.id)
                            +   "=" +   en((o=n.options[n.selectedIndex]).value||o.text);
                }
        }
    }
    return vars
};

So, it's basically doing the same thing for each input type, just appending a key value pair as a string. That gives me the feeling it should be consolidated. We can even see at least the key part, vars+= "&" + en(n.name||n.id), is exactly the same for each one.

It seems a case like this is a limitation of the language: where we are using a switch inside of a loop for a shared instruction of logic, but each case varies specifically enough to cause redundancy. Sometimes, we just vary slightly, where sometimes, we have other distinguishing factors, and sometimes we have to step that logic inside of another loop. At any rate, however, it's essentially the same logic, and should be simplified, either in the methodology and/or in the language's constructs.

So, before I go any further, does anybody have a better method?

Because, now, I have another function element.getFormDataObject because, while that function returned all my data, it was in a string, but what if I need to modify that data? So this function does basically the exact same thing, but returns an object. It didn't make sense to me to always return an object and then translate it into a string because that would be unnecessary overhead when you just need a string. I traded functional overhead for asset overhead. (This one can't do the hard return if it detects a file upload because it assumes you need the data if you called it.)

element.getFormDataObject   = 
function( )
{   var data={};var form=this,i=0,n,o;

    while(form=form.parentNode){if('FORM'===form.tagName)break}
        form=form.getElementsByTagName('*');

    while(n=form[i++])
    {   switch((n.type||'').toLowerCase())
        {   case 'radio':
            case 'checkbox':
                if(n.checked)
                    data[n.name||n.id]=n.value;
            break;
            break;
            case 'select-multiple':
                for(var k=0;o=n.options[k++];)
                    if(o.selected)
                        (data[n.name||n.id]=data[n.name||n.id]||[]).push(o.value||o.text);
            break;
            default:
                switch(n.tagName)
                {   case'INPUT':
                    case'TEXTAREA':
                        data[n.name||n.id]=n.value;
                    break;
                    break;
                    case'SELECT':
                        data[n.name||n.id]=(o=n.options[n.selectedIndex]).value||o.text
                }
        }
    }
    return data
};

Again, this is a case of redundancy. It's like we need to be able to specify the template for each case and datatype and just issue one simple logic instruction inside one loop.

Additionally, I still actually have to have element.toFormData to translate the object into the string.

element.toFormData      = 
function( )
{   var text="",en=encodeURIComponent;
    for(var k in this)
    {   if(typeof this[k]==='string')
            text+=  "&" +   en(k)
                +   "=" +   en(this[k])
        else
            for(var i in this[k])
                text+=  "&" +   en(k)+"["+i+"]"
                    +   "=" +   en(this[k][i])
    }
    return text
};

So, we have a lot of redundant logic all throughout the implementation.

Would really appreciate any thoughts!!!

LetterEh · Accepted Answer

A good way of shrinking duplicate code in JS is through the creation of functions.
A second is through functional techniques, which limit the amount of loop-management boilerplate you need.

In the example below, you'll see that the size of the two functions I've created (getFormData and buildFormEncodedString) are both rather small.
Of course, they're also delegating out to a number of named-helpers, typically 1-5 lines long, and the total file-size has increased.

Many of those helpers, themselves can be removed, using higher-order functions

// instead of
function isSelected (option) { return option.selected; }
function isChecked (input) { return input.checked; }
function isDisabled (input) { return input.disabled; }

inputs.filter(isChecked).filter(isDisabled);


// higher-order function
function is (trait) {
  return function (obj) { return obj[trait]; };
}
// implementation of "not" should be straightforward, once "is" is understood
inputs.filter(not(is("disabled"))).filter(is("checked"));

Even switching to ES6-syntax, and using only destructuring and arrow-lambdas could add clarity and cut down on excess reading:

// instead of
function appendKeyValueToObject (object, pair) {
  var key = pair[0];
  var value = pair[1];
  object[key] = value;
  return object;
}

// ES6 allows
function appendKeyValueToObject (object, [key, value]) {
  object[key] = value;
  return object;
}

// instead of
inputs
.filter(function (input) {
  return input.type !== "file";
})
.map(function (input) {
  return [input.name, input.value];
});

// ES6 allows
inputs.filter(input => input.type !== "file").map(input => [input.name, input.value]);

In any case, a lot of work is now being done, with fewer moving parts to worry about.

You mentioned that you didn't want to incur "overhead" by having a function parse the form, and another function transform the data... ...but the overhead you're talking about (from a performance standpoint) is super-minimal (compared to, say, running an OpenGL game at 60fps, using canvas). It also comes with the added bonus of being able to pop that data into any transform you'd like, for any reason (multipart form upload, JSON upload, et cetera). By forcing your function to do multiple things (grab the element, find the nearest form ancestor, find all elements whether they are for input or layout, pull out all key/value pairs, push them into strings), you have completely precluded the ability to write the reusable functions which you wanted in the first place.

Have a look at the included example, and hopefully, the benefits of this level of separation, and more set-based techniques will become apparent.

initializePageListeners();

function initializePageListeners () {
  var trigger = query("#trigger-collection");
  var jsonArea = query("#print-data");
  var uriArea = query("#print-uri");

  trigger.onclick = function () {
    var form = query("#PersonForm");
    var data = getFormData(form);
    jsonArea.value = JSON.stringify(data);
    uriArea.value = buildFormEncodedString(data);
  };
}


function getFormData (form, keepFiles) {
  var inputs = queryAll("input", form);
  var scalarInputs = inputs.filter(isInputActive).filter(isNotFile);
  var textareas = queryAll("textarea", form);
  var selects = queryAll("select", form);

  var inputValues = concat(scalarInputs, textareas).map(extractInputNameValuePair);
  var inputFiles = !keepFiles ? [] : inputs.filter(isFile).map(extractInputFiles);
  var selectValues = selects.map(extractNameValueSets);

  var formData = buildFormData([inputValues, inputFiles, selectValues]);
  return formData;
}

function buildFormData (inputs) {
  return inputs.reduce(concat, []).reduce(appendKeyValueToObject, {});
}

function buildFormEncodedString (data) {
  var encodedString = Object.keys(data)
    .map(function (key) { return [ key, data[key] ]; })
    .map(function (pair) {
      var key = pair[0];
      var value = pair[1];
      return Array.isArray(value) ? formEncodePairs(key, value) : formEncodePair(key, value);
    }).join("&");

  return encodedString;
}

function formEncodePair (key, value) {
  return [key, value].map(encodeURIComponent).join("=");
}

function formEncodePairs (key, values) {
  return values.map(function (value) { return formEncodePair(key, value); }).join("&");
}

function appendKeyValueToObject (obj, pair) {
  var key = pair[0];
  var value = pair[1];
  var keyExists = (key in obj);

  if (keyExists) {
    obj[key] = concat(obj[key], value);
  } else {
    obj[key] = value;
  }
  return obj;
}

function extractNameValueSets (select) {
  var multiple = select.type === "select-multiple";
  var name = select.name;
  var options = slice(select.options);
  var selectedOptions = options.filter(isSelected).map(function (option) { return option.value || option.text; });

  return [ name, multiple ? selectedOptions : selectedOptions[0] ];
}

function extractInputFiles (input) {
  var key = input.name || input.id;
  var files = input.files;
  return [key, files];
}

function extractInputNameValuePair (input) {
  var value = input.value;
  var key = input.name || input.id;
  return [key, value];
}

function isSelected (option) { return option.selected; }
function isFile (input) { return input.type === "file"; }
function isNotFile (input) { return !isFile(input); }
function isInputActive (input) {
  var canCheck = /radio|checkbox/i.test(input.type);
  return canCheck ? input.checked : true;
}

function query (selector, node) {
  var root = node || document;
  return root.querySelector(selector);
}

function queryAll (selector, node) {
  var root = node || document;
  return slice(root.querySelectorAll(selector));
}

function concat (a, b) {
  return [].concat(a).concat(b);
}

function slice (arrLike, start, end) {
  return [].slice.call(arrLike, start, end);
}

* { box-sizing: border-box; }

#print-data, #print-uri {
  display: block;
  width: 100%;
}


  
    Name
    first: 
    last: 
  
  
    Gender
    male: 
    female: 
  
  
    Language Experience
    JavaScript
      
    
    CSS
      
    
  
  
    Paradigm Experience
    Select all that apply

JavaScript: Optimizing against Redundancy (Loop + Switch + Key/Value)

Answers (1)

Related Questions