2

I am using a function to increment values in an array depending on the value passed to it.

function depth(x) {
    var dep = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];

    if (x < 10) {
        var deps = dep[0];
        dep[0] = deps + 1;

    } else if (x >= 10 && x < 20) {
        var deps = dep[1];
        dep[1] = deps + 1;

    } else if (x >= 20 && x < 30) {
        var deps = dep[2];
        dep[2] = deps + 1;

    } else if (x >= 30 && x < 40) {
        var deps = dep[3];
        dep[3] = deps + 1;

    } else if (x >= 40 && x < 50) {
        var dep2 = dep[4];
        dep[4] = deps + 1;

    } else if (x >= 50 && x < 60) {
        var deps = dep[5];
        dep[5] = deps + 1;

    } else if (x >= 60 && x < 70) {
        var deps = dep[6];
        dep[6] = deps + 1;

    } else if (x >= 70 && x < 80) {
        var deps = dep[7];
        dep[7] = deps + 1;

    } else if (x >= 80 && x < 90) {
        var deps = dep[8];
        dep[8] = dep2 + 1;

    } else if (x >= 90 && x < 100) {
        var deps = dep[9];
        dep[9] = dep2 + 1;

    } else if (x >= 100 && x < 110) {
        var deps = dep[10];
        dep[10] = deps + 1;


    } else if (x >= 110 && x < 120) {
        var deps = dep[11];
        dep[11] = deps + 1;

    } else if (x >= 120 && x < 130) {
        var deps = dep[12];
        dep[12] = deps + 1;

    } else if (x >= 130 && x < 140) {
        var deps = dep[13];
        dep[13] = deps + 1;

    } else if (x >= 140 && x < 150) {
        var dep2 = dep[14];
        dep[14] = deps + 1;

    } else if (x >= 150 && x < 160) {
        var deps = dep[15];
        dep[15] = deps + 1;


    } else if (x >= 160 && x < 170) {
        var deps = dep[16];
        dep[16] = deps + 1;

    } else if (x >= 170 && x < 180) {
        var deps = dep[17];
        dep[17] = deps + 1;

    } else if (x >= 180 && x < 190) {
        var deps = dep[18];
        dep[18] = dep2 + 1;

    } else if (x >= 190 && x < 200) {
        var deps = dep[19];
        dep[19] = dep2 + 1;

    } else {
        var dep2 = dep[10];
        dep[20] = dep2 + 1;

    }

}

My question is, is there a cleaner way to do this or would I need to write an else if statement for each possible variable?

JAL
  • 21,295
  • 1
  • 48
  • 66
Nixsy
  • 33
  • 3
  • one is remove all else, you can do comparison in switch see [this answer](http://stackoverflow.com/questions/17679292/shortening-a-javascript-if-else-structure) – Grijesh Chauhan Feb 01 '14 at 11:26
  • 1
    Yeah... There's a pattern, that's for sure. Round x to lowest 10. – JAL Feb 01 '14 at 11:28

4 Answers4

3

Besides your else statement, there is a clear pattern here. So the easiest thing to do is have an exception for that and then treat the rest together.

function depth(x) {
    var dep = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
    if (x >= 200) { // case for anything above 200
       return dep[20] = dep[10] + 1;
    }
    dep[Math.floor(x/10)]++;
}

Your else case made it a little messier, but in general, this is a 1 liner kind of thing.

Niall Paterson
  • 3,580
  • 3
  • 29
  • 37
  • This seems to have solved it perfectly, http://i.imgur.com/4REemZM.jpg thank you for your help – Nixsy Feb 01 '14 at 11:39
  • Great stuff! Hope it helped. – Niall Paterson Feb 01 '14 at 11:40
  • Why not using `Math.max()` instead of the `if` statement? – Jivan Feb 01 '14 at 11:48
  • @Jivan it's not incrementing del[20] if it's over 200, it's adding one to dep[10] and assigning that to dep[20]. That's why yours is still wrong.. I think it's messy, but considering the dep[10] in there, it's hard to see how it could be made much better. – Niall Paterson Feb 01 '14 at 11:54
  • @Niall ok got it, thanks - I don't really see another (cleaner) way in that case. – Jivan Feb 01 '14 at 11:56
  • @Jivan yeah agreed, it's almost a pity, because it makes it a lot more awkward – Niall Paterson Feb 01 '14 at 11:56
  • Yeah. I hesitate between "this was a typo" and "Nixsy wanted to see if there's a way to automate a pattern when there is a behavioral exception such as this one" – Jivan Feb 01 '14 at 12:05
2

A problem of repeating patterns

The problem is that you repeat yourself a lot because the code in your if/else statements share lots of similarities.

When you find such similarities in places of your code, it is often useful to "merge" them in some way.

For example, you could note that the code in your statements is perfectly identical except the 10, 20, 30 etc values. Then, try to determine a way to make those 10, 20, 30, 40, depend on an unique variable with which you'll replace them.

In that case, it's easy to guess because each time, the array's key that you're working with is equal your number divided by 10. With the help of some methods of the JavaScript's built-in object Math, this is a matter of some single-line statements:

function depth(x) {
    var dep = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];

    var nb = Math.floor(x / 10);
    nb = Math.max(nb, 20);
    dep[nb]++;
}

Some resources about Math functions:

Math.max on Mozilla Developer Network

Math.floor on Mozilla Developer Network

JavaScript Math built-in object

Jivan
  • 21,522
  • 15
  • 80
  • 131
  • Yeah, it's easy enough to see a pattern and than simplify it into this short statement, that's one of the most important tasks of a programmer :-) – Michal Artazov Feb 01 '14 at 11:34
  • 1
    you can simplify it event more - instead of the last 2 lines you can type `dep[nb]++` – Michal Artazov Feb 01 '14 at 11:36
  • Haha great, I was just doing that while you were typing – Jivan Feb 01 '14 at 11:38
  • Just documenting here, as I mentioned below, that `Math.max` doesn't solve this. The aim isn't to increment dep[20] if it's over 200, it's adding one to dep[10] and assigning that to dep[20]. Very different. – Niall Paterson Feb 01 '14 at 11:55
  • You're perfectly right Niall. But I'll let my answer as is, because I can't find any way better than yours to address this. – Jivan Feb 01 '14 at 12:07
1

Try this:

var index = Math.floor(x/10);
var deps = dep[index];
dep[index] = deps + 1;
barak manos
  • 29,648
  • 10
  • 62
  • 114
  • 1
    This only works if x is a multiple of 10. Look at the other answers to see the correct way to get from x to an index in the array. – Tibos Feb 01 '14 at 11:45
1

Something like that should do the trick but since your dep is local and never returned the function will do nothing.

function depth(x) {
    var dep = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
    if(x < 200) {
      var index = Math.floor(x/10);
      dep[index]++;
    } else {
      dep[20] = dep[10] + 1;
    }
}

Anyway, as you probably guessed, when you see a pattern you can probably reduced it to something simpler.

If you have ranges of 10, it means you can divide the numbers by 10 to get an index. You have to use Math.floor because 1/2 => 0.5 and It will try to add the value to the index "0.5" which doesn't exists. Math.floor will truncate the number to the lowest value. In our case, 0,1,2,3,4....

There is no global rule, but when you see a pattern that repeat itself, you just have to find what is common between each case and you'll be able to find ways to simplify most situations by converting values to something that will be common to all ifs.

Loïc Faure-Lacroix
  • 13,220
  • 6
  • 67
  • 99