5

I have conditionals like this:

if (foo == 'fgfg' || foo == 'asdf' || foo == 'adsfasdf') {
// do stuff

}

Surely there's a faster way to write this?

Thanks.

Mark
  • 32,293
  • 33
  • 107
  • 137
  • 1
    No... not really... If it was foo == null || foo == undefined || foo == 0 it could be shortened to if (!foo)... but if your comparing foo against 3 strings, then you need 3 different comparisons. No way around it. – Zoidberg Jun 21 '10 at 02:26
  • 1
    Your second one can be shortened to "if (true)", it can't be fgfg and asdf at the same time :) – Dumb Guy Jun 21 '10 at 02:29
  • Whoops lol, what was I doing, nevermind on that second one. – Mark Jun 21 '10 at 02:32
  • 1
    possible duplicate of [short hand for chaining logical operators in javascript?](http://stackoverflow.com/questions/2932131/short-hand-for-chaining-logical-operators-in-javascript). But please *don't even look* at the `Object.prototype.in` addition in the accepted answer, because: 1. Extending `Object.prototype` is a *really bad* idea. 2. `in` is a reserved word (the `in` operator)... – Christian C. Salvadó Jun 21 '10 at 02:33
  • OK, it looks like the answer is 'no'. I don't feel very hot about regex or arrays, they don't feel more readable or faster to type to me. And they can only be slower. I was hoping there would be a commonly accepted shorthand, but since there is none, then no is probably the answer? – Mark Jun 21 '10 at 02:36
  • 1
    How fast something is to type is an odd metric to consider. I would gladly take a permanent 50% reduction in my typing speed in exchange for being able to code as fast as I type. – Ori Pessach Jun 21 '10 at 02:55
  • Also a duplicate of http://stackoverflow.com/questions/2218844/how-to-make-this-if-statement-shorter – Sean Hogan Jun 21 '10 at 03:14

8 Answers8

6

You might consider a switch-case statement

switch(foo) {
  case "fgfg":
  case "asdf":
  case "adsfasdf":
    // ...
}

It's not really any shorter, but could be more readable depending on how many conditions you use.

erjiang
  • 44,417
  • 10
  • 64
  • 100
4
if (/^(fgfg|asdf|adsfasdf)$/.test(foo)) {

or:

if (["fgfg", "asdf", "adsfasdf"].indexOf(foo) != -1) {

Cross-browser support for Array.indexOf is still limited. Also, these are faster to write, probably not faster to run.

Matthew Flaschen
  • 278,309
  • 50
  • 514
  • 539
4

I would keep the conditionals the way they are. Any clever way of shortening them would make the code less idiomatic and less readable.

Now, if you do care about readability, you could define a function to do the comparison:

if( foo_satisfies_condition(foo) ) {
  // ...
}

Or:

if( is_month_name(foo) {
  // ...
}

If you give the function a name that faithfully describes what it does, it will be easier to understand the intent of the code.

How you implement that function would depend on how many comparisons you need. If you have a really large number of strings you're comparing against, you could use a hash. The implementation details are irrelevant when reading the calling code, though.

Ori Pessach
  • 6,777
  • 6
  • 36
  • 51
4

No need for using indexOf or a regex if you just use a hash table:

var things = { 'fgfg' : 1, 'asdf' : 1, 'asdfasdf' : 1 };
if ( things[foo] ) { 
    ... 
}
friedo
  • 65,762
  • 16
  • 114
  • 184
  • I think this would perform quite well. – ChaosPandion Jun 21 '10 at 02:40
  • Honestly, I think this is less readable than just writing conditionals, and I wouldn't think it would be faster to write. – Sasha Chedygov Jun 21 '10 at 03:51
  • 1
    If you think that's less readable, I sure wouldn't want to be tasked with working on your code. :) Nothing bugs me more than seeing a hugely complex conditional glued together with a dozen boolean operators when a lookup table would make things so much easier. – friedo Jun 21 '10 at 04:46
1

Here's a easy way:

String.prototype.testList = function(lst) {
 lst = lst.split('|');
 for(var i=0; i<lst.length; i++){
  if (this == lst[i]) return true;
 }
 return false;
};

To use this function, you can just do this:

if (foo.testList('fgfg|asdf|adsfasdf')) {

You can also rename testList to whatever you want, and change the delimiter from | to anything you want.

Dumb Guy
  • 3,336
  • 21
  • 23
  • I would invert the sense of the test, eg: String.prototype.hasToken = function(token) { return this.split(/\s+/).indexOf(token) != -1; }. – Sean Hogan Jun 21 '10 at 03:22
0

Depending on the situation you could do..

//At some point in your code
var vals = new Array('fgfg', 'asdf', 'adsfasdf');
//...
if(vals.indexOf(foo) >= 0)
Anthony
  • 12,177
  • 9
  • 69
  • 105
  • Matthew Flaschen's is better. – Anthony Jun 21 '10 at 02:26
  • You should look at it in terms of performance as well, when the options list grows, regex performance goes down the drain, `.indexOf()` remains a very low cost, if you instantiate the array once it's better of course. – Nick Craver Jun 21 '10 at 02:29
0

Ternary operator looks good if you like and has else

Community
  • 1
  • 1
Niklas Rosencrantz
  • 25,640
  • 75
  • 229
  • 424
0

Use tilde (~) for a shorter expression

if (~["fgfg", "asdf", "adsfasdf"].indexOf(foo)) {
    //do stuff
}
Anton Kiyatkin
  • 1