Reputation:
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
Reputation: 96346
I wouldn't touch the code.
There are many posted codes here, but still yours is the most readable.
Upvotes: 1
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
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
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
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
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
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
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
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
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
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
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
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
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