14
\$\begingroup\$

Is there a better way to do this without using if or case statements?

var val = ui.value,
$box = $('#message');
switch(true) {
  case (val > 50):
    $box.hide();
    animations.animateBox($box.html('you have chosen XN'));                        
    break;
  case (val < 50):
    $box.hide().delay(300);
    animations.animateBox($box.html('you have chosen VN'));
    break;
  default:
    $box.hide();
    animations.animateBox($box.html('you have chosen PN'));
}
\$\endgroup\$
4
  • \$\begingroup\$ Although switch can be used with expressions, you should better only use literal/constant values. \$\endgroup\$
    – Gumbo
    Dec 22, 2011 at 12:32
  • 10
    \$\begingroup\$ That's a very wrong use of a switch statement \$\endgroup\$
    – jAndy
    Dec 22, 2011 at 12:33
  • \$\begingroup\$ Would it be simpler to go with a if..else if..else construct. I don't understand your use of switch here. \$\endgroup\$
    – Abbas
    Dec 22, 2011 at 12:34
  • \$\begingroup\$ Conditional logic. Better way to do this without using if or case. ಠ_ಠ \$\endgroup\$
    – Ben Brocka
    Dec 22, 2011 at 20:07

5 Answers 5

18
\$\begingroup\$

This completely replaces your switch statement by using inline if statements conditional operator:

$box.hide().delay(val < 50 ? 300 : 0);
var s = val > 50 ? 'XN' : (val < 50 ? 'VN' : 'PN');
animations.animateBox($box.html('you have chosen ' + s));

the first line could be replaced by these two arguably more readable lines that omit the call to delay() function when not applicable:

$box.hide();
val < 50 && $box.delay(300);

Since @GregGuida pointed out that this code is less readable I suppose I can make it more readable by better formatting it. And I'll use the suggestion of replacing the first line with two of them:

$box.hide();

// only delay animation when value < 50
val < 50 && $box.delay(300);

var s = val > 50 ? 'XN' :
       (val < 50 ? 'VN' :
                   'PN');
// set HTML
animations.animateBox($box.html('you have chosen ' + s));

Same code but visually less chaotic (even without comments it would look less chaotic) and much much more readable. At least much more readable than OP's original code. That I'm sure of. Readability is of course an argumentative disposition. But I'll leave that to others.

\$\endgroup\$
9
  • 2
    \$\begingroup\$ The only thing I would change us abstracting out the conditions into variables. Like var isXN=val>50; but that's just me. +1 \$\endgroup\$
    – J. Holmes
    Dec 22, 2011 at 12:45
  • 5
    \$\begingroup\$ The problem with this solution is that it is 100 times harder to read. this is code review not code golf \$\endgroup\$
    – Greg Guida
    Dec 22, 2011 at 18:46
  • \$\begingroup\$ @GregGuida: That may be, hence the two line replacement suggestion for the first line. But for the rest it does remove the excessive if statement and switch statement that OP was after. Quote: Is there a better way to do this without using if or case statements Unquote. My answer is exactly that. No switch nor if statement if we say that inline if is not an if... At least it doesn't seem like one. \$\endgroup\$ Dec 22, 2011 at 19:20
  • \$\begingroup\$ Your cleanup makes it much more readable, I guess I just disagree with the "no if" part of the question. If ever there was a place to use if ... if else ... else this is it. Also not to be a JS snob, but its called a conditional operator \$\endgroup\$
    – Greg Guida
    Dec 22, 2011 at 19:39
  • \$\begingroup\$ Ternary's still technically an if. You're just not actually writing "if" in the code. The "without if" bit of the question is kind of absurd though. \$\endgroup\$
    – Ben Brocka
    Dec 22, 2011 at 20:09
14
\$\begingroup\$

You must not use conditional statements in combination with switch... that is THE DEVIL... because switch/case only interpretates values !

Infact, what you did there equals

switch( true ) {
    case true/false:
       break;
    case true/false:
       break;
    etc.
}

That is ohhh-so-wrong! Only use switch if you have well defined states/values which you want to check for. You totally should go with a if/else statement there.

if( val > 50 ) {
}
else if( val < 50 ) {
}
else { // val equals 50
}
\$\endgroup\$
7
  • 1
    \$\begingroup\$ You are writing this as if it's technically wrong use switch the way Amit did, however technically it's not wrong at all. It is of course wrong, because it's a bad code style and affects readability. \$\endgroup\$
    – RoToRa
    Dec 22, 2011 at 15:09
  • 1
    \$\begingroup\$ I agree with @RoToRa. Switch statement like that works. If it was wrong it wouldn't work. Whether it's safe to do it this way or whether it aids readability it's a different question and an argumentative one. \$\endgroup\$ Dec 22, 2011 at 19:22
  • \$\begingroup\$ @jAndy: when you say case true/false it's actually the same with every case statement. The variable in switch is either of some value or it's not. The same with these expressions... So technically switch statement with case expressions is argumentative in its nature at best. \$\endgroup\$ Dec 22, 2011 at 19:23
  • \$\begingroup\$ @RobertKoritnik: Well, a switch statement is intended to check a variable for multiple values/states. The usage in the OPs code is totally confusing and potentially dangerous. Since his expressions can only end up beeing true or false you will never be able to have more than 2 cases. So again, this is pure evil. \$\endgroup\$
    – jAndy
    Dec 22, 2011 at 21:09
  • 1
    \$\begingroup\$ You could quite easily have as many true/false cases as you want. The switch statement in JS is much more flexible than Java's or C's; it has no requirement that case values be unique, and doesn't require a constant/literal for the case expression. It'll just start at the first case that evaluates equal to the switch expression. The switch (true) thing is ugly, and an abuse of switch, and not optimizable into a jump table, but it's quite valid JS -- and any interpreter that can't deal with it is broken. \$\endgroup\$
    – cHao
    Dec 23, 2011 at 19:36
2
\$\begingroup\$

You can try something like:

var val = ui.value,
    $box = $('#message');
    var choice = (val > 50) ? 'XN' : ((val < 50) ? 'VN' : 'PN');

    if(val < 50) {
        $box.hide().delay(300);
    } else {
        $box.hide();
    };

    animations.animateBox($box.html('you have chosen ' + choice));

You could remove the IF statement altogether if it weren't for the delay().

\$\endgroup\$
4
  • \$\begingroup\$ What does .delay(0) ? \$\endgroup\$
    – Stefan
    Dec 22, 2011 at 12:38
  • \$\begingroup\$ var delayAmount=(val<50)?300:0; $box.hide().delay(delayAmount); \$\endgroup\$
    – Tom B
    Dec 22, 2011 at 12:40
  • \$\begingroup\$ delay stops execution for the number of milliseconds indicated. api.jquery.com/delay \$\endgroup\$
    – Tom B
    Dec 22, 2011 at 12:42
  • \$\begingroup\$ Here's a simple example. jsfiddle.net/Ccugd \$\endgroup\$
    – Ayman Safadi
    Dec 22, 2011 at 12:50
1
\$\begingroup\$

You can use the conditional operator of you want to avoid if and switch. You can reduce some repetiton in the code using a variable for the chosen product:

var val = ui.value,
var box = $('#message');
box.hide().delay(val < 50 ? 300 : 0);
var product =
  val > 50 ? 'XN' :
  val < 50 ? 'VN' :
  'PN';
animations.animateBox(box.html('you have chosen ' + product));

You should still consider if the code is more readable using if statements:

var val = ui.value,
var box = $('#message');
box.hide();
if (val < 50) {
  box.delay(300);
}
var product;
if (val > 50) {
  product = 'XN';
} else if (val < 50) {
  product = 'VN';
} else {
  product = 'PN';
}
animations.animateBox(box.html('you have chosen ' + product));
\$\endgroup\$
1
\$\begingroup\$

My suggestions:

  1. use ternary operators;
  2. use single char;
  3. for prevent type casting, compare defined values with undefined;
  4. use logical "and" (&&) instead of if.

Code snippet:

var val = ui.value,
    box = $('#message'),
    chr = 50 > val ? 'V' : (50 < val ? 'X' : 'P');
box.hide();
50 > val && box.delay(300);
animations.animateBox(box.html('You have chosen ' + chr + 'N'));
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.