Shortening a Javascript if-else structure

Question

The code I have is:

 var level = function (d) {
    if (value(d) > median + stdev) {
        return 1;
    } else if (value(d) > median) {
        return 2;
    } else if (value(d) > median - stdev) {
        return 3;
    } else {
        return 4;
    }
 };

Is there a nicer way of doing this?

This is a better question for http://codereview.stackexchange.com. — Matt Ball, Jul 16 '13 at 14:29
a nested ternary operator seems possible, though will be harder to read — everconfusedGuy, Jul 16 '13 at 14:33
What are your criteria for "nicer"? Is there a reason you are re-evaluating value(d) up to three times? — symcbean, Jul 16 '13 at 14:39
That is easy to read and understand. Why would you need to modify it? — zzzzBov, Jul 16 '13 at 21:22

6502 · Accepted Answer · 2013-07-17T14:24:05.673

15

Of course calling value(d) multiple times is something you can avoid.

Also you can shorten a little using the symmetry:

  var level = function (d) {
    //
    //               -std    median  +std
    // ----------------|-------|-------|------------------
    // 4444444444444444 3333333 2222222 111111111111111111
    //
    var i = Math.floor((median - value(d)) / stddev) + 3;
    return Math.max(1, Math.min(4, i));
  };

Probably not a good idea for a real project however... I didn't test but I wouldn't be surprised to find this code slower than the original in your question and for sure I find it harder to maintain.

Note that excluding one-shot throwaway scripts normally code is written once and read many times (for maintenance like improvements or debugging), thus "easier to read" is normally much more important than "easier to write".

When shorter means "easier to read" is a good thing, when it starts meaning "harder to read" it's not.

edited Jul 17 '13 at 14:24

answered Jul 16 '13 at 14:43

6502

112,025
15
165
265

awesome man! very nice documentation/representation, I remember you from python answer. – Grijesh Chauhan Jul 16 '13 at 16:39
what is "symmetry" ? :) – Jul 16 '13 at 17:35
@EricFromSouthPark: The simplification comes from the fact that the two intervals are equal, so dividing the difference between the value and the median by the width of the interval you already get a numeric integer code that only needs clamping. – 6502 Jul 16 '13 at 19:12
7

the ascii art is nice, but i do not see how the code is "nicer". it is more "clever" and fewer lines, but it uses division and functions from Math, and generally does not work with a nominal scale ( https://en.wikipedia.org/wiki/Level_of_measurement#Nominal_scale ). it is not faster to execute. refactoring the code is no longer straight forward, e.g. if you want other values/another type for the values or just include other/more cases... this is code golf ( http://en.wikipedia.org/wiki/Code_golf ), but no improvement in code quality. – mnagel Jul 16 '13 at 20:07
3

@mnagel: I agree. Please double-check what is the question title. – 6502 Jul 16 '13 at 21:09
@6502: the title and the closing request in the question differ somewhat, and everyone concentrated on "short" instead of "nice". this might or might not be what the original author wanted, i cannot say. i find your answer insightful, but i would not apply the refactoring to my code (i would copy the docstring, though). my comment provides concrete arguments about the problematic aspects i see and it is intended to make you think what you care about in your code. altogether, this is a good answer imho, but not an answer i wanted to leave without a comment. – mnagel Jul 16 '13 at 21:28
I do not want to downvote this answer, because that is kind of what was asked for, but this is still **very** bad advice on every level. It is (probably) slower and near unmaintainable. – Chronial Jul 16 '13 at 21:50
@Chronial: I agree. Except for the multiple calls to `value(...)` I'd keep the original code. – 6502 Jul 16 '13 at 22:14

score 7 · Answer 2 · answered Jul 16 '13 at 14:53

7

To complete the set, here is the switch way referenced by @austin :

var level = function (d) {
  var d = value(d) - median;
  switch (true) {
  case d > stdev : return 1;
  case d > 0:      return 2;
  case d > -stdev: return 3;
  default:         return 4;
  }
};

answered Jul 16 '13 at 14:53

HBP

15,685
6
28
34

is this allow in case `< == >` ? if yes then +, else I will come back to down-vote you :) – Grijesh Chauhan Jul 16 '13 at 16:36
I am confident that you will have no need to down vote. It is a surprising construct when you first see it. The definition of `case` is actually an expression not a literal value. This can be used to advantage in sometimes. – HBP Jul 16 '13 at 16:43
I am a c programmer, its really surprising to me!, is it only possible in JavaScript? or New versions of Java allows this too? Because once back in time I was wondering that case can be strings in Java. – Grijesh Chauhan Jul 16 '13 at 16:48
1

It is certainly easier to do in an interpretive language like JavaScript. As you know in C the case value needs to be a literal integer value. A quick check shows that in Java the case values must be integer or string constants. – HBP Jul 16 '13 at 17:56
I've seen this in other languages as well, though I've heard that it's a performance killer (at least in JavaScript). If this has to be run many times, your original if/else is probably faster. I wish I could do this in other languages, but none of the languages I use regularly (C/C++/C#/Java) allow it. – Darrel Hoffman Jul 17 '13 at 02:09

score 4 · Answer 3 · answered Jul 16 '13 at 14:38

4

This solution is nicer, but I would not recommend using it in production as it is kind of confusing:

4 - [median + stdev, median, median - stdev].filter(function(e, i, a) {
    return value(d) > e;
}).length

answered Jul 16 '13 at 14:38

Amberlamps

39,180
5
43
53

2

yeah but +1 for surely using the least amount of bytes to store on my HD – jenson-button-event Jul 16 '13 at 14:39
@jenson-button-event: Those are the most important bytes ;) – Amberlamps Jul 16 '13 at 14:40
3

I didn't downvote but for sure this and @6502 answers are a bit far to be immediately understandable. ;D – Alberto Zaccagni Jul 16 '13 at 14:46

score 3 · Answer 4 · answered Jul 16 '13 at 14:40

You can calculate the difference between the value and the median, which makes the comparisons simpler:

function level(d) {
  var n = value(d) - median;
  if (n > stdev) {
    return 1;
  } else if (n > 0) {
    return 2;
  } else if (n > -stdev) {
    return 3;
  } else {
    return 4;
  }
};

You can also write it using the conditional operator instead of if statements:

function level(d) {
  var n = value(d) - median;
  return n > stdev ? 1 :
    n > 0 ? 2 :
    n > -stdev ? 3 :
    4;
  }
};

Whether this is nicer or not is a matter of taste, but it's shorter.

score 2 · Answer 5 · answered Jul 16 '13 at 14:37

"nicer" way? No, not really.

Alternate way(s) - yes, plentiful.

One possibility is storing the condition and result in an array

var levelFunctions = [
  { func: function(d){ return value(d) > median + stdev; }, val:1},
  { func: function(d){ return value(d) > median ; }, val:2},
  { func: function(d){ return value(d) > median - stdev; }, val:3},
  { func: function(d){ return true; }, val:4}
];

Then just enumerating that list as part of the function

var level = function (d) {
    for(var i=0;i<levelFunctions.length;i++){
       if(levelFunctions[i].func(d))
           return levelFunctions[i].val;
    }
 };

A bit easy to extend than your original, but by [diety] its ugly as sin!

If you really wanted to use an array, you should use one like @amberlamps did. — Bergi, Jul 16 '13 at 14:58

Alberto Zaccagni · Answer 6 · 2013-07-16T14:51:15.673

I don't see much room for improvements which could guarantee the same readability of the starting case. If you are really returning integers, and they aren't there just for the sake of this example, I would suggest you to return something more meaningful.

You could for sure compute value(d) just once

var level = function (d) {
  var dValue = value(d);
  if (dValue > median + stdev) {
    return 1;
  } else if (dValue > median) {
    return 2;
  } else if (dValue > median - stdev) {
    return 3;
  } else {
   return 4;
  }
};

Also, you might want to avoid multiple returns, or maybe you don't, for me they're the same, each has advanges/disadvantages:

var level = function (d) {
  var dValue = value(d),
      code = 4;
  if (dValue > median + stdev) {
    code = 1;
  } else if (dValue > median) {
    code = 2;
  } else if (dValue > median - stdev) {
    code = 3;
  } 
  return code;
};

If you assign a meaningful name to code you are giving even more information to the guy who is reading your code.

Bergi · Answer 7 · 2013-07-16T14:50:19.677

I'd recommend to avoid multiple calls to value:

function level(d) {
    var diff = value(d) - median;
    if (diff > 0) {
        if (diff > stdev)
            return 1;
        else
            return 2;
    else
        if (diff > -stdev)
            return 3;
        else
            return 4;
}

Also I've nested the if-else-statements in a (hopefully) more meaningful structure - that depends on your usecase though. Might be more helpful if you returned values like -2, -1, 1 and 2 instead or something. A ternary operator could save you some writing, but it's not necessarily getting clearer.

Alternatively, some maths could help you:

function level(d) {
    var diff = value(d) - median;
    return 2 + (diff > 0 ? -.5 : .5) * (Math.abs(diff) > stdev ? 3 : 1);
}

Though it leads to 3 instead of 4 in case value(d) === median-stdev. See @6502's answer on how to avoid that.

score 2 · Answer 8 · answered Jul 17 '13 at 02:10

2

Well if we're going for short, creative solutions...

var level = function(d){
    d = value(d);
    return +(d<=median+stdev)+ +(d<=median)+ +(d<=median-stdev) + 1
}

answered Jul 17 '13 at 02:10

mowwwalker

16,634
25
104
157

i find this solution quite 'creative' – A. Wolff Aug 01 '13 at 12:39

score 1 · Answer 9 · edited May 23 '17 at 12:33

1

You could use a switch statement (Mozilla Developer Network docs).

Edit: If you think it's impossible to do ranges in a switch statement, here's an answer to a similar question.

edited May 23 '17 at 12:33

Community

1
1

answered Jul 16 '13 at 14:29

austin

5,816
2
32
40

Not (easily) for greater-than/less-than checks. – Matt Ball Jul 16 '13 at 14:29
I don't think a 30 times slower technique is particularly "nice". – Dahaka Jul 16 '13 at 14:37
You should include a code snippet to show how that can be used here. It's a good technique that you pointed to. – Drahcir Jul 16 '13 at 14:38
1

It will be longer whereas the OP wanted shorter. It is also clever which can be considered a bad thing outside StackOverflow answers – Esailija Jul 16 '13 at 14:38

score 1 · Answer 10 · answered Jul 16 '13 at 14:38

This doesn't remove the if/else structure but makes the code a bit cleaner:

var level = function (d) {
    var delta = value(d) - median;
    if (delta > stdev) {
        return 1;
    } else if (delta > 0) {
        return 2;
    } else if (delta > -stdev) {
        return 3;
    } else {
        return 4;
    }
 };

It has the added benefit of calling value(d) only once.

Pebbl · Answer 11 · 2013-07-16T23:28:27.363

Another option — ignoring the usefulness of maths in this instance — is to do away with the if statements altogether. I generally prefer this approach to that of using ternary operators. I tend to think this is more readable than having multiple if/else constructs (for simple situations), but that is only because I am versed in the ways of JavaScript's logical operators. I can fully understand how odd it might look to those learning, or people who code in strange and foreign languages where 1 && 3 === TRUE and not 3

var level = function (d) {
  d = value(d);
  return ((d > median + stdev) && 1) 
      || ((d > median)         && 2) 
      || ((d > median - stdev) && 3)
      || 4
  ;
}

A further possible optimisation — unique to this question — would be to remove median from the comparisons, however, this would most likely affect the readability:

var level = function (d) {
  d = value(d) - median;
  return ((d > + stdev) && 1) 
      || ((d > 0)       && 2) 
      || ((d > - stdev) && 3)
      || 4
  ;
}

score 1 · Answer 12 · answered Jul 18 '13 at 20:26

1

var level = function(d){
    var n = value(d) - median, values = [];
    values[stdev]  = 1;
    values[0]      = 2;
    values[-stdev] = 3;
    return values[n] ? values[n] : 4;
};

values could be pulled out of the function scope if desired.

answered Jul 18 '13 at 20:26

Matt

11
1

score 1 · Answer 13 · answered Jul 20 '13 at 11:47

1

I wouldn't touch the code.

There are many posted codes here, but still yours is the most readable.

answered Jul 20 '13 at 11:47

Karoly Horvath

94,607
11
117
176

score 0 · Answer 14 · answered Jul 16 '13 at 14:53

0

Try this tho ...

  var reduceCalcVal=value(d);   //reduce repeated calculation
  var cheats=new Array( reduceCalcVal > median + stdev
    ,reduceCalcVal  > median, reduceCalcVal > median - stdev);
    n=(cheats.indexOf(true)==-1)?4:cheats.indexOf(true)+1;
    alert(n)

answered Jul 16 '13 at 14:53

internals-in

4,798
2
21
38

More succinctly : d = value(d) - median; return [d > stdev, d > median, d > -stdev, true].indexOf (true) + 1; – HBP Jul 16 '13 at 15:00

Shortening a Javascript if-else structure

14 Answers14

Linked