2
\$\begingroup\$

Building a form where values can be entered in either pounds or kilos.

Script is...

function onlyNumber(fld) {
     if(fld.value.match(/[^0-9.]/)) {
     fld.value=fld.value.replace(/[^0-9.]/g,'');
     }
}
function convertUnit(sense, value) {
    retValue = 0;
    if (isNaN (value)) { alert ('Non-numeric value');  return 0; }
    value = parseFloat (value);
    var factor = 0.4536;
    if (sense == 'kg2lb') {
         retValue = value/factor;
         retValue = Math.round(retValue);
    }
    else if (sense == 'lb2kg') {
         retValue = value*factor;
         retValue = Math.round(retValue);
    }
    return retValue;
}

Works fine but I have to repeat this on every input:

onblur="this.form.kg_va.value=convertUnit('lb2kg',this.value);"     
onkeyup="onlyNumber(this);" onfocus="if(this.value == '0') { this.value = ''; }" 

Is there some way to simplify the onblur code ?

For example, is there a way to have the function focus on the same var pairs? The var pair being lb_FOOBAR and kg_FOOBAR.

And what about the input functions that are exactly the same?

onkeyup="onlyNumber(this);" onfocus="if(this.value == '0') { this.value = ''; }"

Any way to avoid having to repeat them for each input? Form will have 12 fields.

\$\endgroup\$
  • \$\begingroup\$ Your function onlyNumber assumes a . for decimal notation. This is not true for most of the world. You could add some logic to that function to do a better guess. Some info \$\endgroup\$ – Jonathan Dec 6 '15 at 22:51
1
\$\begingroup\$

Instead of inlining the script, use addEventListener instead. To retrieve the elements, you'd need something like getElementsByClassName or querySelectorAll to return an array-like structure of elements. You loop through them, and attach a listener each.

var formInputs = Array.prototype.slice.call(document.querySelectorAll('.form-input'));
formInputs.forEach(function(element){
  element.addEventListener('blur', function(){
    // blur code
  });
  element.addEventListener('keyup', function(){
    // blur code
  });
});

The Array.prototype.slice.call is to convert the array-like structure returned by querySelectorAll into a real array. That's so I can use forEach. But a regular for loop will do, since a NodeList (the array-like structure) has a length property.

Now instead of alert and return, why not throw an error instead?

if(isNaN(value)) throw new Error('Non-numeric value');

Also, instead of putting all your conversions in a single function and sending a "sense", why not create a function for each kind of conversion? It's more verbose, but you don't have to pile up everything in one function.

\$\endgroup\$
  • \$\begingroup\$ I'd suggest using element.addEventListener('input' fn); instead of keyup, as this will break when a user pastes input \$\endgroup\$ – Jonathan Dec 6 '15 at 22:57
  • \$\begingroup\$ I disagree on throwing the error, I don't think most users will look in their console to see why nothing happens. \$\endgroup\$ – Jonathan Dec 6 '15 at 23:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.