user1899082
user1899082

Reputation:

Shortening a Javascript if-else structure

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?

Upvotes: 17

Views: 776

Answers (14)

Karoly Horvath
Karoly Horvath

Reputation: 96346

I wouldn't touch the code.

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

Upvotes: 1

Matt
Matt

Reputation: 11

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.

Upvotes: 1

6502
6502

Reputation: 114599

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.

Upvotes: 15

mowwwalker
mowwwalker

Reputation: 17392

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
}

Upvotes: 2

Pebbl
Pebbl

Reputation: 36075

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
  ;
}

Upvotes: 1

internals-in
internals-in

Reputation: 5048

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)

Upvotes: 0

HBP
HBP

Reputation: 16063

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;
  }
};

Upvotes: 7

Alberto Zaccagni
Alberto Zaccagni

Reputation: 31590

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.

Upvotes: 2

Bergi
Bergi

Reputation: 665554

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.

Upvotes: 2

Guffa
Guffa

Reputation: 700910

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.

Upvotes: 3

ARRG
ARRG

Reputation: 2496

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.

Upvotes: 1

Amberlamps
Amberlamps

Reputation: 40498

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 

Upvotes: 4

Jamiec
Jamiec

Reputation: 136239

"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!

Upvotes: 2

austin
austin

Reputation: 5876

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.

Upvotes: 1

Related Questions