I've recently adopted a new coding style for switch
statements in JavaScript (well, technically TypeScript), and I'm really curious to hear what you all think about it!
Before I discuss the style, let me explain the problem I'm trying to solve. It's not uncommon for switch statements to have somewhat-repeated code between cases, but with variations in each case such that you can't easily abstract it. Let's take this example, written in ES5 (I promise there's a reason why).
switch (body.type) {
case 'isBasic':
var entry = getBasicEntry(body.id);
console.log('Processing basic entry "' + entry.name + '"');
doBasicProcessing(entry);
break;
case 'isCustom':
var entry = getCustomEntry(body.id);
console.log('Processing custom entry "' + entry.name + '"');
doCustomprocessing(entry);
break;
default:
throw new Error('Unknown flag ' + myFlag);
}
Cool, it's a basic switch statement and it works fine. Now, let's convert this to ES2015+.
switch (body.type) {
case 'isBasic':
const entry = getBasicEntry(body.id);
console.log(`Processing basic entry "${entry.name}"`);
doBasicProcessing(entry);
break;
case 'isCustom':
const entry = getCustomEntry(body.id);
console.log(`Processing custom entry "${entry.name}"`);
doCustomprocessing(entry);
break;
default:
throw new Error(`Unknown flag ${myFlag}`);
}
Uh-oh, we now get a SyntaxError
exception when we run this code! The problem here is that const
variable name entry
cannot be declared twice in the same scope. Because of how scoping rules work, the entire switch
statement is one single scope.
We could solve this by naming each entry
with a different variable name, but honestly I find that kind of annoying. We could also create a helper function for each case, but that feels excessive to me for such short case statements.
So here's what I figured out, and I've started using it in all my code:
switch (body.type) {
case 'isBasic': {
const entry = getBasicEntry(body.id);
console.log(`Processing basic entry "${entry.name}"`);
doBasicProcessing(entry);
break;
}
case 'isCustom': {
const entry = getCustomEntry(body.id);
console.log(`Processing custom entry "${entry.name}"`);
doCustomprocessing(entry);
break;
}
default: {
throw new Error(`Unknown flag ${myFlag}`);
}
}
I wrap each case statement in {}
, which creates a new scope on a per-case basis. This solves the problem, and I think is kind of elegant myself. It's not very common though.
What do you all think? Does this seem like a good approach? Do you have a different approach that you use? I'd love to hear it in the comments!
Update:
Thank you all for the discussion so far, it's been great! I ended up creating a new module called conditional-reduce that automates another approach I hadn't thought of before:
const { reduce } = require('conditional-reduce');
console.log(reduce('dog', {
dog: () => 'Dogs are great pets',
cat: () => 'Cat\'s are also great'
})); // Prints "Dogs are great pets"
console.log(reduce('bird', {
dog: () => 'Dogs are great pets',
cat: () => 'Cat\'s are also great'
})); // Throws 'Invalid conditional value "bird"' exception
const { curry } = require('conditional-reduce');
const dogReducer = curry({
dog: () => 'Dogs are great pets',
cat: () => 'Cat\'s are also great'
});
console.log(dogReducer('dog')); // Prints "Dogs are great pets"
console.log(dogReducer('bird')); // Throws 'Invalid conditional value "bird"' exception
Thank you @avalander and @john_papa for the discussion that prompted this!
Top comments (41)
ATM I prefer not writing switch statements and instead use conditional return statements like
Although this makes most sense when moving the conditions to distinct constants first, like:
Of course this depends on the use case. When having the necessity for more complex conditionals it might make sense to move them to distinct variables anyways and that's when this pattern shines...
This is a good one too. In the case of TypeScript, we do loose a bit of type checking (you can leave the
default
case off if the possible values are anenum
IIRC), but this is good too.I've really grown fond of using this approach in React specifically, since we can't have normal conditionals inside of JSX:
Why not
let entry;
at the top of the switch block?If you're trying to set a variable from multiple place, it's not a constant.
Adding a scope, requires another stack frame. Ie increasing overhead.
I'd like to see the profiling run that shows that an extra stack frame here and there appreciably impacts performance. I promise it doesn't matter. Optimizing for readability is usually a better return on your investment than trying to optimize for performance. Also keep in mind the compiler is doing a lot of very smart optimization for you. It's better at it than we are. Of course, things that obviously matter like N+1 database queries obviously matter. Stack frames in JavaScript though? I would not worry unless you're doing something very novel.
Depends. Probably not significant unless it's really hot code (ex parser stap).
That likely wouldn't be a significant deal breaker, just worth considering.
My main counter is, why increase complexity, making the code both harder to read and maintain when defining entry using 'let' is much simpler/easier to understand.
To each their own. I bias heavily toward simple/maintainable code even of it's not 100% semantically perfect. Ie avoid adding complexity unless it's 100% necessary.
I think defining using a
let
is the harder to read and maintain option than my option, and going that path increases complexity TBH.In this example I'm not trying to set one variable from multiple places. Rather, I'm trying to set two different variables once each that never change. They actually are constants.
Each variable also has a different type in TypeScript too. There is a base class between them, but explicitly specifying the base class on the
let
and then casting in the function calls adds extra scaffolding that I don't think is necessary and want to avoid.I'm not worried about adding another stack frame TBH. Once upon a time there was a performance cost to extra stack frames (and to function calls, try/catch/etc.), but V8 has been optimized enough these days that this only matters for extremely hot paths. And if this were a hot path, I'd write it pretty differently anyways, after taking the time to measure it and prove it's a hot path in need of optimization.
Yeah, a place I've done this is in writing switch statements in reducers. The extra scope also closes over any intermediate values you have so you don't have to coordinate variable names between blocks. I do this routinely for all switch statements all the time for this reason. The semantics are more useful and obvious with the braces.
I would actually write it like this:
because different things warrant different variable names.
But if you cannot come up with different names, maybe these aren't actually different things and you should aim to solve the issue with a unified approach that doesn't even require a switch statement.
I don’t agree with this, for three reasons:
1) there are more possibilities than things are the same or are different. It’s also possible, as is the case here, that they are mostly, but not completely, the same
2) we use short-hand variables all the time that are repeated. How many callbacks have
err
as the first argument? Are these all the same thing? No. But we do it anyways and we consider that a best practice.3) crafting a unified approach for this sort of case would require extra boilerplate, resulting in an over-engineered solution that obfuscates what’s actually going on.
1) Actually, that things are either handled the same or different is a literal dilemma without a middle ground. So in the context of their current handling they are either identical or different, regardless of their other similarities.
2) using the same shorthands twice in the same context is a recipe for confusion unless they actually mean the exactly same thing.
3) That's why I asked this as an open question. If things are actually so similar that you can handle them without too many extra cases, you should obviously do so – and if not, your variable names should reflect the difference.
I still disagree, for the reasons I stated above. I think we’ll just have to agree to disagree, but thank you for sharing your perspective!
Thanks for taking your valuable time to raise and discuss this point.
I usually don't use switch statements at all, I prefer a having a dictionary object with a handler function for each case.
Maybe it's just an idiom I brought from Python, but I think it's more natural to compose functions with this pattern than with switch statements.
I created a little module based on this conversation, and of course gave you a shout-out in the README :D
It's at npmjs.com/package/conditional-reduce
Code looks like this:
I realized, thanks to @john_papa 's post below, that this approach also allows us to return a value in the process, so we can inline it in other expressions (or React's JSX)!
Wow, that went quick :D
You could also consider having the function
conditionalReduce
curried and pass the dictionary as the first argument, to make it easier to reuse.Also a great idea. I added a new helper so that folks can do either. Thanks again, this makes things cleaner!
Oh cool, I’ve never seen this approach before! Gonna noodle on this one cause I’ve got a feeling there’s a tiny helper library in here waiting to be built to make the missing handler logic even more elegant and baked in. Thank you for sharing.
I also prefer thing method,
Reading this again (it's good, well done Bryan!).
I've been thinking about switch statements that collect a value so it can be returned at the end. imagine this ... which does not work for a few reasons.
The idea is to collect the result, then return it at the end. I'm not fond of this type of code with the
let
up top and then making sure each case handles a value. Any thoughts on what you do here?This is a great insight! I combined this with a post above and created a little helper module: npmjs.com/package/conditional-reduce
Code looks like:
So with that ... the code would be somehtng like this, for the example you and I used first. RIght?
A few syntax issues aside, yes ;-)
It would look like:
yeah, i figured the dev.to editor wouldn't protect me and that you'd forgive those mistakes :)
I prefer a variation of what you have. This is what I do in my reducers and I think it translates well in components.
I feel that this has 3 advantages:
1st, you can easily see what's going on in the switch statement and go from there.
2nd, by returning, you don't have to worry about missing a
break;
3rd, essentially they are different processes and entry may have the same name, but may or may not differ in type for custom and basic...or if implementations for processing them them may change this will allow you to easily update that with separated functions.
This is definitely my second top choice, but I prefer my other approach honestly. I really don't like functions that are 2-3 lines long, unless they're dense and hard to read. IME, having a lot of small functions strung together can get hard to follow, since you're having to jump all over the file to trace the path of execution.
To be clear though, this is not a strong preference at all, it's pretty mild, and your method is definitely my second pick :)
All of these solutions are great, but they could benefit from having compile-time exhaustiveness checking via the never type. I describe that here: dev.to/cubiclebuddha/is-defensive-...
This works when each case is discreet (having its own scope), ie each case terminates the switch with a break. But one of the features of switches is that things "fall through" each case until they hit a break. I've always liked that aspect of switches, even though it's often overlooked and can make it more confusing to read/grasp.
Just curious if setting each case in a scope like your suggested style specifically prevents that fall-through or breaks (renders unexpected behavior) code between switches due to the scope not carrying over.
Oh man, I love when I do a little further research and find out that the thing I'm talking about has a huge debate associated with it.
en.wikipedia.org/wiki/Fallthrough_...
en.wikipedia.org/wiki/Duff%27s_device
So it may be the case that your suggested syntax adds extra reinforcement for fall-through to be avoided as an intentional design practice.
You beat me to my own argument 😉
That said, I myself have also used fall-through statements on rare occasions.
That that said (is that a thing?), scoping each case using
{}
doesn't prevent you from falling through case statements anyways.Great tip! I've landed on a similar practice informed by eslint's no-case-declaration rule.
Typically I default to
case
s without blocks, and add them when that rule reminds me to.That’s a nice approach too, as it also removes the “whoops I didn’t realize I did that” effect. Good to know, thanks!
I love this! I had this exact problem last week and I fell into the same solution. It really bothered me that I had so much duplicate code.
Glad you wrote this up. Highly readable (which I love) and a great tip for everyone.
Nicely done Bryan!
As someone who comes from a long history of C++, this is how I do it in JS anyway. Forcing scope in c++ is something I've done in the middle of a code block, even, so it's pretty natural in JS to do the same.
That’s a good point!