JavaScript - simplify/shorten code

Question

I have the following function which I would like to simplify with a for loop maybe but don't know how to do it. any help will be much appreciated. Basically, if the field value is 0 or null then my total value (field) should be 0 otherwise if the field value is from 1 until 1000 then the total value becomes 5000. For every 1000 (i.e. 1001 until 2000) my total value should increase by 50 i.e. 5050. This should continue until the field value reaches 200000 and the total is 50000.

function calc56() {
    var56 = parseFloat(document.getElementById('units56').value - 0);

    if (var56 == '' || var56 == 0) {
        document.getElementById('amount56').value = 0;
    }
    else if (var56 < 1000) {
        document.getElementById('amount56').value = 5000;
    }
    else if ((var56 > 1000) && (var56 <= 2000)) {
        document.getElementById('amount56').value = 5050;
    }
    else if ((var56 > 2000) && (var56 <= 3000)) {
        document.getElementById('amount56').value = 5100;
    }
}

Thanks in advance.

I'm voting to close this question as off-topic because it is asking for an improve ment to working code. ask on codereview.stackexchange.com — Daniel A. White
– Daniel A. White, Commented Jan 21, 2016 at 11:07
Few things about your code. If var56 is less than 0 you will still get 1000 and you say 1 to 1000 but your condition is var56 < 1000 which means it will only work to 999 and 1000 will not trigger any of the conditions. — gothical
– gothical, Commented Jan 21, 2016 at 11:19

Stephan Bijzitter · Accepted Answer · 2016-01-21 14:50:13Z

1

function calc56() {
    var el = document.getElementById('units56'); //reference the dom element
    var val = +el.value; //convert to float

    if (!val) { //if no value, leave untouched

    } else if (val < 0) { //if value is less than 0, make it 0.
        el.value = 0;
    } else { //otherwise, calculate new value
        var mod = Math.floor(val / 1000); //calc how many 1000s fit in the value
        el.value = mod * 50 + 5000; //use at least 5000, and add 50 for every 1000
    }
}

I would suggest you also change the name of the function, as it's not very useful. However, this code right here should be the most efficient you can get while still keeping it readable.

If you need more clarification, feel free to ask in a comment!

edited Jan 21, 2016 at 14:50

answered Jan 21, 2016 at 11:12

Stephan Bijzitter

4,6455 gold badges28 silver badges45 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Alexander Elgin Over a year ago

Check my solution (2 tiny functions, 7 lines of code, a live snippet that anyone can check) if you think that your code is the most efficient

Stephan Bijzitter Over a year ago

Well, since your answer contains more function calls, duplicates a DOM query and is able to trigger a DOM repaint even without changing the value, @AlexanderElgin, I do not have to think long to see my answer is much more efficient ;-)

Collectives™ on Stack Overflow

JavaScript - simplify/shorten code

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related