9 ms·
I prefer early return pattern. Which would look like this: const user = { firstName: "Seán", lastName: "Barry", email: "my.address
by kvczor 5y ago
I prefer early return pattern.
Which would look like this:
const user = {
firstName: "Seán",
lastName: "Barry",
email: "my.address@email.com",
number: "00447123456789",
};
if (!user) {
throw new Error("User must be defined.");
}
if (!user.firstName) {
throw new Error("User's first name must be defined");
}
if (typeof user.firstName !== "string") {
throw new Error("User's first name must be a string");
}
return user;
- kvczor 5y agoLinting got messed up in the comment above, but in the actual code this is nicely readable.
- deleted 5y ago[deleted]
- elyseum 5y agoNo need to ‘return’ when you throw an error, but your approach is valid IMO: if structure / readability is that important, refactor the switch to it’s own method with only if checks in it. Hard to make that simpler and more readable.
- notyourday 5y agoIf you don't have a 'return' there's nothing to say that some time later some junior developer would not modify throw to be something else or forget that throw does not return.
- dfee 5y agoThis will prevent the JR dev from making that mistake: https://eslint.org/docs/rules/no-fallthrough https://eslint.org/docs/rules/no-fallthrough
- BiteCode_dev 5y agoThat's what tests are for.
- cced 5y agoI think the issue here though, is that for certain cases such as form field validation, you want all issues to be returned at once. Using the switch method or similar message packages allow you to inform your users that have N issues with a page in 1 request as opposed to N.
- planb 5y agoThat’s not how throw works. And that’s exactly why the switch(true) pattern should not be used: novice users not fully knowledgeable about every specification of a language should not fail to understand such a basic piece of code. There’s not even a line difference to using if statements correctly (as others in the comments have demonstrated) The one thing an if statement does is checking if something is ‚true‘. I don’t understand why anyone would use a ‚switch‘ here apart from showing how clever they are.
- Sayrus 5y agoEither you will break, thus exiting the switch-case block or you will fall-through. The fall-through behavior won't do what you want: > the script will run from the case where the criterion is met and will run the cases after that regardless if a criterion was met. Using the switch method won't allow you to return several errors while the simple ifs method described earlier could accumulate the errors and return later. The switch method is elegant though.
- thomas_moon 5y agoThe early return pattern was the most effective single piece of advice I received from a senior dev on how to make my code more readable. You end up with clearer code paths than any other pattern I have seen so far and way less indentation making things look (and probably perceived) as less complex. Pair it with naming your complex and chained expressions and suddenly you have some seriously readable code. So far, I have never seen a valid scenario where a switch statement is actually any better than if.
- mekster 5y agoUnwanted conditions should be handled before handling the process in a peaceful condition. Not sure it makes much sense to indent everything within "if" and if you forget to "else", you've just potentially hidden a bug.
- brundolf 5y agoUnpopular opinion: I think early-returns make code less readable if you don't put the rest of the function in an else clause. You lose visual parallelism, and suddenly instead of following a tree down to a series of leaves where every leaf terminates, you have to have the full context to know whether or not a line of code may not execute in some cases. For example: function foo(x) { if (x == null) { return null; } x += 2; return x; } I can't just look at "x += 2;" and know whether or not it's conditionalized. I have to have the full context including the early-return, and then reason about the control flow from there. Whereas: function foo(x) { if (x == null) { return null; } else { x += 2; return x; } } Here I can tell just from the else-block that this is one possibility which will execute if the other one does not, and vice-versa. Their indentation is the same, cementing their relationship. If I want to know whether this block will execute I need only look at the if()'s, not their contents.
- cryptonector 5y agoOn the contrary, if early returns allow you to outdent a larger portion of code then it's quite a readability win. As long as all the early return cases are bite-sized and the last case long-ish, it's a readbility win.
- btown 5y agoI agree; `if (` is much more well-known and "grokkable" to engineers at practically any level than `case` - and exactly the same number of characters. There's no need to bring a sledgehammer when the nail is perfectly handled by a normal hammer.
- layer8 5y agoI agree, but the problem presumably is the “else”.
- 0xcoffee 5y agoI've also seen code written as: _ = !isDefined(user) && throw new Error("User must be defined."); _ = !isString(user.firstName) && throw new Error("User's first name must be a string"); But I while it is concise, I can also understand why people prefer regular if statements. Doesn't seem to be valid JS, can't remember where I got it from
- CloselyChunky 5y agoIn my opinion this pattern is better if you write it like this: _ = isDefined(user) || throw new Error("user must be defined") This reads way more natural for me. "A user is defined OR throw an error"... I've also seen this in Perl (`do_something() || die()`) and shell scripts (`grep -q || die "not found"`).
- pxx 5y agoIn perl you would want to use `or` instead of `||` to take advantage of the low precedence, so you can type things like `dostuff $foo or die` which is logically 'do stuff, and if it fails, die' as opposed to 'die if $foo is false', which you'd get with ||
- lhorie 5y agoIt's a stage 2 proposal, which technically can be used today via babel[0] (though personally, I don't recommend using stuff below stage 4) [0] https://babeljs.io/docs/en/babel-plugin-proposal-throw-expressions https://babeljs.io/docs/en/babel-plugin-proposal-throw-expre...
- IggleSniggle 5y agoMy preference for this is to use an assertion function, eg in nodejs: assert(isDefined(user),”User must be defined”) // throws if Falsy
- d1sxeyes 5y agoThis pattern is quite common in React to do conditional rendering: {isLoading && <Loading />}
- qudat 5y agoAgreed! I have three rules for writing functions: - Return early - Return often - Reduce levels of nesting https://erock.io/2019/07/11/three-rules-for-refactoring-functions.html https://erock.io/2019/07/11/three-rules-for-refactoring-func...
- 1_player 5y agoEarly return is my silent gauge to tell if a piece of code has been written by a junior or a senior software engineer. I still see far to many snippets with multiple levels of indentation, where each if branch is for the happy path, and if they were converted to early returns you could flatten the whole thing to 1 indentation level, at the expense of requiring negated boolean expressions which aren't as readable.
- eh9 5y agoI would usually agree, but the suggested code would allow for multiple errors to be shown before they’re thrown.
- high_byte 5y agoswitch true is a cool new trick to me, but early return is the way to go.
- tengbretson 5y agoI've grown to dislike the early return pattern. It's pitched as a way to reduce visual complexity by reducing levels of indentation, but I don't actually find that to be a benefit in most cases. Reducing indentation is just a trick to shove more cyclomatic complexity into a function without it triggering your sensibilities to implement proper abstractions.
- qalmakka 5y agoSo that has got a name? I always instinctively wrote code like that, because it's much more easier to understand.
- mwkaufma 5y ago1000X this.
- darepublic 5y agoswitch true is not a substitute for early return it is a replacement for the equivalent written in if chain