281

Option 1 - switch using return:

function myFunction(opt) {
  switch (opt) {
    case 1: return "One";
    case 2: return "Two";
    case 3: return "Three";

    default: return "";
  }    
}

Option 2 - switch using break:

function myFunction(opt) {
  let retVal = "";

  switch (opt) {
    case 1: 
      retVal = "One";
      break;
    case 2: 
      retVal = "Two";
      break;
    case 3: 
      retVal = "Three";
      break;
  }

  return retVal;
}

I know that both work, but is one more of a best practice? I tend to like Option 1 - switch using return best, as it's cleaner and simpler.


Here is a jsFiddle of my specific example using the technique mentioned in @ic3b3rg's comments:

let SFAIC = {};

SFAIC.common = {
  masterPages: {
    cs: "CS_",
    cp: "CP_"
  },
  contentPages: {
    cs: "CSContent_",
    cp: "CPContent_"    
  }
};

function getElementPrefix(page) {
  return (page in SFAIC.common.masterPages)
    ? SFAIC.common.masterPages[page]
    : (page in SFAIC.common.contentPages)
      ? SFAIC.common.contentPages[page]
      : undefined;
}

To call the function, I would do so in the following ways:

getElementPrefix(SFAIC.common.masterPages.cs);
getElementPrefix(SFAIC.common.masterPages.cp);
getElementPrefix(SFAIC.common.contentPages.cs);
getElementPrefix(SFAIC.common.contentPages.cp);

The problem here is that it always returns undefined. I'm guessing that it's because it's passing in the actual value of the object literal and not the property. What would I do to fix this using the technique described in @ic3b3rg's comments?

Audwin Oyong
  • 2,247
  • 3
  • 15
  • 32
Code Maverick
  • 20,171
  • 12
  • 62
  • 114

3 Answers3

354

A break will allow you continue processing in the function. Just returning out of the switch is fine if that's all you want to do in the function.

ic3b3rg
  • 14,629
  • 4
  • 30
  • 53
  • 8
    So, given the example in my question, the answer is yes. But, if you have a function where you need to keep going, obviously a break would be what you would use. – Code Maverick May 24 '11 at 17:15
  • 12
    @Mark Costello's answer made me thank a bit more about your question. I think you're looking for a general "best practice" guideline, but in the specific example you gave, the best practice is `return {1:"One",2:"Two,3:"Three"}[opt];`. If you need the default then it would be `var o={1:"One",2:"Two,3:"Three"}; return opt in o?o[opt]:"";` – ic3b3rg May 24 '11 at 17:36
  • @ic3b3rg - I've edited my question with my specific example trying to utilize your technique of `return (opt in o) ? o[opt] : "";`, but it always returns the default in my specific case. – Code Maverick May 24 '11 at 18:51
  • There was a typo in my code (missing the 2nd `"` in `"Two"`) but it works for me... here's a simple test: `var o={1:"One",2:"Two",3:"Three"},opt=2; alert(opt in o?o[opt]:"");` – ic3b3rg May 24 '11 at 19:12
  • I wasn't using your example, just the technique. Look at my question and click the link to my jsFiddle to see what I'm talking about. – Code Maverick May 24 '11 at 19:19
  • gotcha... try `getElementPrefix("cs")`, etc. – ic3b3rg May 24 '11 at 19:28
  • Well, if you look in my specific example, I can't do that, as both `masterPages` AND `contentPages` have the same properties, just different values. – Code Maverick May 24 '11 at 19:31
  • if you want to go 2-deep then you need 2 parameters to your function and the logic is then: `function getElementPrefix(type,page){return (type in SFAIC.common) && (page in SFAIC.common[type]) ? SFAIC.common[type][page] : undefined;}` – ic3b3rg May 24 '11 at 19:35
  • Yea, that's what I was thinking I'd have to do. Thanks. – Code Maverick May 24 '11 at 19:42
  • If you need to continue processing, it might make sense to split the function into two: the first with `switch` + `return`; and the second which uses the first one and continues processing of the result. – phadej Dec 22 '14 at 17:17
  • So if I return in each case, so I dont need to break every case, correct? – anhtv13 Oct 23 '17 at 07:04
40

Neither, because both are quite verbose for a very simple task. You can just do:

const result = ({
  1: 'One',
  2: 'Two',
  3: 'Three'
})[opt] ?? 'Default'  // opt can be 1, 2, 3 or anything (default)

This, of course, also works with strings, a mix of both or without a default case:

const result = ({
  first: 'One',
  'sec-ond': 'Two',
  3: 'Three'
})[opt]  // opt can be 'first', 'sec-ond' or 3

Explanation:

It works by creating an object where the options/cases are the keys and the results are the values. By putting the option into the brackets you access the value of the key that matches the expression via the bracket notation.

This returns undefined if the expression inside the brackets is not a valid key. We can detect this undefined-case by using the nullish coalescing operator ?? and return a default value.

Example:

console.log('Using a valid case:', ({
  1: 'One',
  2: 'Two',
  3: 'Three'
})[1] ?? 'Default')

console.log('Using an invalid case/defaulting:', ({
  1: 'One',
  2: 'Two',
  3: 'Three'
})[7] ?? 'Default')
.as-console-wrapper {max-height: 100% !important;top: 0;}
leonheess
  • 16,068
  • 14
  • 77
  • 112
  • 1
    what is this method called? – Sourav Singh Apr 13 '21 at 13:49
  • 2
    @SouravSingh I don't think it has a name. Tbh I just came up with it when I wasn't satisfied with the solutions here. I guess I'd call it something like "object-switching" – leonheess Apr 14 '21 at 09:38
  • 1
    Great use of nullish operator! – BeHFaR Dec 02 '22 at 09:09
  • 2
    If you have any logic or calculations, this method will cause every option to be evaluated every time. However, by making every option a function instead of a value (e.g. `() => 'three'`), you can avoid this issue: `({...options})[1]()`. Or, for default behavior, `({...options})[1]?.() ?? 'Default'`. If some options might return null and you don't want to overwrite that: `(({...options})[1] ?? (() => 'Default'))()`. – Daniel May 10 '23 at 19:28
  • I wonder why this awesome snippet of code has no name – Mayeenul Islam Jul 31 '23 at 09:22
10

It depends, if your function only consists of the switch statement, then I think that its fine. However, if you want to perform any other operations within that function, its probably not a great idea. You also may have to consider your requirements right now versus in the future. If you want to change your function from option one to option two, more refactoring will be needed.

However, given that within if/else statements it is best practice to do the following:

var foo = "bar";

if(foo == "bar") {
    return 0;
}
else {
    return 100;
}

Based on this, the argument could be made that option one is better practice.

In short, there's no clear answer, so as long as your code adheres to a consistent, readable, maintainable standard - that is to say don't mix and match options one and two throughout your application, that is the best practice you should be following.

Mark Costello
  • 4,334
  • 4
  • 23
  • 26
  • 3
    The best practice in that example is `return foo == "bar";` – ic3b3rg May 24 '11 at 17:31
  • 11
    I apologize if I'm annoying you but in that case I'd still simplify: `return foo == "bar" ? 0 : 100;` or even `return [100,0][foo == "bar"];`. – ic3b3rg May 24 '11 at 17:40
  • 5
    @ic3b3rg - Shouldn't that be: `return [100,0][+(foo == "bar")];` ? – Queue Jan 24 '13 at 19:58
  • 3
    @Queue You're correct in that the boolean should be converted to integer, but I'd do it this way: `return [100,0][foo == "bar" & 1];` – ic3b3rg Jan 24 '13 at 21:32
  • 1
    @ic3b3rg - That is beautiful; but how does it compare speedwise to using a unary plus? – Queue Jan 29 '13 at 22:37
  • 9
    @ic3b3rg && Queue - How would you like to maintain somebody elses code using such tricks ? (Trust the precompiler to speed-optimize stuff like that) – T4NK3R Sep 29 '18 at 07:38