-
Notifications
You must be signed in to change notification settings - Fork 26.8k
Closed
Description
Was reading through the style guide, but couldn't find anything about this. Instead of
var callback;
if (callback) {
callback();
}
I've started using
var callback;
callback && callback();
Which I haven't found any issues with. Would this style guide frown upon this?
adriancmiranda, ErikCupal and gusgard
Activity
bishopZ commentedon Feb 21, 2013
+1
ssorallen commentedon Feb 21, 2013
This leads to an error in JSLint and a warning in JSHint, "Expected an assignment or function call and instead saw an expression”. As JSLint explains its
expr
option:The warning is useful and worth enabling as it does catch typos like JSLint explains. The line you wrote,
callback && callback();
is an expression and not a function call. This may be contrived, but take code similar to your example:JSLint will give the same warning for the code, and this time it caught a typo:
callback
is not actually being called, it's missing parentheses. This code would raise the same warning for the same reason:The second example would likely be more quickly noticed by the developer, but again JSLint/JSHint is able to point it out. I find using an explicit
if
more readable for the next developer too. The intention of the conditional in that case is unmistakable whereascallback && callback();
or something like it is not as straightforward.There is nothing "wrong" with writing
callback && callback();
in the sense that it will run just fine. However, I am a fan of keeping it out of the style guide because endorsing a pattern that looks the same as a typo to tools can lead to frustrating debugging and code that is harder to read for the next developer.timofurrer commentedon Feb 21, 2013
I think it is more readable to make an
if
instead of this shorter statement, although it is not wrong!Like @ssorallen pointed out!
medikoo commentedon Feb 22, 2013
I make such if's, a one liners:
Still lint will scream, but I take such approach as much more justified (and readable) than hacky
cb && cb()
.hshoff commentedon Feb 25, 2013
I added a link to this discussion so folks can come back and read through this and add to the discussion.
prashantpalikhe commentedon Mar 11, 2014
I always prefer to use if statements while developing. When the project is built, uglifyjs will short-circuit the if statements to use the construct like yours.
netpoetica commentedon Apr 6, 2014
I don't think either one adequately covers safely attempting to run your callback
is what I use to be safe. I don't think a simple null check is a good idea especially when the arguments are variable or when you intend to use arguments[arguments.length - 1] convention:
Otherwise:
收藏页
yisibl commentedon Nov 1, 2019
What about
callback?.()
?ljharb commentedon Nov 1, 2019
@yisibl thats equivalent to
if (callback != null) { callback(); }
, so that’d be fine (once eslint supports parsing that syntax, and once this guide permits it)[other styles] add link to conditional callback discussion. fixes air…