views:

69

answers:

3

I've got a Javascript (dojo) function that I'm using to auto-populate a form. The idea is to pass in a JS object with keys for the form field ids, and values that help define how to fill in the field.

It works, but it feels a bit ugly to me (the switch statement, the objects within objects syntax). Any thoughts on how can I improve* this code?

/**
 * Fill in the form with passed in values
 *
 * @param {Object} defaults 
 *
 * Can be used to pass in default values to the form. Expects an object like this:
 *   {<field id>: {type: '<field type>', value:'<value>'}
 *
 *   for example:
 *   {
 *   paymethod: {type: 'select', value:'Account Transfer'},   // html select
 *   fundsource: {type: 'combo', value:'Corporation Account'} // dijit.comboBox
 *   }
 */
function fillDefaults(defaults) {
    var props;
    for (var field in defaults) {
     props = defaults[field];
     switch (props['type']) {
      // so far only select and combo have been implemented 
      // others will be added as needed
      // and maybe grouped depending on how they work 
      // (e.g. all dijits together, <input> with <select>, etc.)
      case 'select':
       dojo.byId(field).value = props['value'];
       dojo.byId(field).onchange()
       break;
      case 'combo':
       dijit.byId(field).attr('value', props['value']);
       break;
     }
    }
}

[*] improve: make prettier, more js-like, more dojo-like, more streamlined

+1  A: 

What does pretty mean? What's js-like?

A switch, although memory intensive, is cleaner if you plan to extend with more objects.

Maybe, instead of a switch, have an object containing your functions:

funcs = {
  select: function(value) {
    dojo.byId(field).value = value;
    dojo.byId(field).onchange()
  },
  combo: function(value) {
    dijit.byId(field).attr('value', value);
  }
};

function payFillDefaults(defaults) {
  var props;
  for(var field in defaults) {
    props = defaults[field];
    if(funcs[props['type']]) {
      funcs[props['type']](props['value']);
    }
  }
}
jameswatts
A switch is memory intensive? Can you justify that statement?
Josh
We ran some tests last year on an A.I. interpreter written in JS. The parser had massive condition chains which were all written with switches to aid readability. After testing if/else vs switch we found if/else to evaluate faster than the switches. However, what with TraceMonkey and Google's V8 my statement may bare less weight now. No justification, just experience. I'm pretty sure I watched a vid a while back of Douglas Crockford talking about it.
jameswatts
A: 

Improved a bit according JSLint standards:

function fillDefaults(defaults) {
    var props, field;
    for (field in defaults) {
        props = defaults[field];
        switch (props.type) {
        case 'select':
            dojo.byId(field).value = props.value;
            dojo.byId(field).onchange();
            break;
        case 'combo':
            dijit.byId(field).attr('value', props.value);
            break;
        }
    }
}
CMS
A: 
Eugene Lazutkin