33

It seems that:

if (typeof a == 'undefined') {
    a = 0;
}

and

(typeof a != 'undefined') || (a = 0)

has the same effect in Javascript.

I really like the second one because it is short, one line code, but is this legal, and cross browser valid? I mean, jslint says it has errors. Should I use it without concerns?

Tamás Pap
  • 16,112
  • 13
  • 65
  • 94
  • 5
    Just a question. What purpose does a complex code serve? – Subir Kumar Sao Sep 28 '12 at 07:23
  • 34
    A slightly better version would be `a = (typeof a !== 'undefined') ? a : 0` – Blender Sep 28 '12 at 07:24
  • 1
    Depends what your definition of *complex code* is – Alexander Sep 28 '12 at 07:25
  • 8
    (typeof a != 'undefined') || (a = 0) is well suited for js "encryption". Advice for lifetime , dont code for yourself only. – Jashwant Sep 28 '12 at 09:34
  • 1
    @Blender: No, that wastes an assignment, which can matter with ES5 and assignment-style property setters. Anyway a || a=default is a common idiom for setting default values in functions/constructors. – Phil H Sep 28 '12 at 09:59
  • First snippet 'if typeof a is undefined, then a is assigned 0'. Second snippet 'if typeof a is not undefined, then the operator short-circuits so nothing happens. Otherwise, so when typeof a is undefined, a is assigned 0 and the expression evaluates to false' Gee I wonder which one takes less thought to understand, is quicker to read correctly (i.e. not assuming that's a `a == 0` ), and is more to the point. – Joren Sep 28 '12 at 10:08
  • This feels very similar to the not so uncommon practice in Perl to have a conditional followed by "or die" if you want the script to terminate immediately if the conditional fails. Choose for yourself if you should consider the perl similarity as a warning or not. – Buhb Sep 28 '12 at 11:07
  • 2
    If you just want one-line code, why not do `if(typeof a == 'undefined) a = 0;` on one line? – Random832 Sep 28 '12 at 12:29
  • 1
    Slightly better version of the second one, that retains equality making it easier to mentally parse: `(typeof a == 'undefined') && (a = 0)`. Not necessarily better than @Blender's version, though. – Izkata Sep 28 '12 at 13:59
  • 1
    Don't favor "short" or "one-line" code for their own benefit. It's fine if something happens to be short, but that should *never* be the intent. The intent should be for *clarity* over length, all the time. – Andrew Barber Oct 11 '12 at 22:18
  • 2
    Liking overly "clever" code is just a phase, experience cures it. :) – simon Oct 18 '12 at 14:16
  • Related: [What a strange syntax?](http://stackoverflow.com/q/6829736/1048572) – Bergi Aug 18 '16 at 21:07

10 Answers10

76

IMHO || (a = 0) is way too similar to || (a == 0) and thus confusing. One day overzealous developer will just "fix it", changing the meaning of your code. And every other developer will have to sit for a while to figure out whether this was your intent or just a simple bug.

And this is in fact what JSLint is trying to say:

Expected a conditional expression and instead saw an assignment.

I avoid using confusing constructs as they hurt readability. a = a || 0; is way more recognizable and similar in meaning.

Tomasz Nurkiewicz
  • 311,858
  • 65
  • 665
  • 652
  • 1
    Even I think developer might mistake it for a typo. – Subir Kumar Sao Sep 28 '12 at 07:24
  • 4
    Incidentally a = a || 0 will be a bit pointless when a is 0 to start with, since 0 is falsy and a will thus be re-set to zero. – Phil H Sep 28 '12 at 09:28
  • 1
    A quick jsperf shows that (typeof a == 'undefined') is as fast as (!a), so there's not even a speed improvement for avoiding the apparent string comparison. – Phil H Sep 28 '12 at 10:00
  • 7
    `a = a || 0` isn't the same. You're doing to get 0 for `false` and `null`, which _might_ not be what the OP wants. – Izkata Sep 28 '12 at 14:01
  • 1
    I might be wrong, but my assumption is that what the OP really wants is var a=0, not a=0. If so, (var a=0) is clearly a variable assignment and cannot be confused with (a==0) – Christophe Oct 05 '12 at 17:23
  • I usually have a ternary with the clause I want. `a = (typeof a != 'undefined') ? a : 0;` Still readable, but also flexible, IMHO at least. – Rakesh Pai Oct 05 '12 at 21:17
  • The usual way of signalling intent is to add an extra set of braces. E. g. writing `if ((x=y))` is pretty common in C. Though the intent should be obvious anyway if the result of the logical operators is not used anywhere. Why would anyone write a statement like `(typeof a != 'undefined') || (a == 0)`? – Tgr Oct 06 '12 at 05:57
  • We don't need to spare file size any more today. So technically there's no reason to write shorter code instead of descriptive. Looking from the maintainability side, descriptive, clear (clean) code always wins! There's no need to show how smart you are knowing all the shortcuts that make you look like a pro. When coding, in the long run lazyness never wins! – RSeidelsohn Mar 10 '13 at 19:36
31

Why not something more simple, like:

a = a || 0;

or

a = a ? a : 0;

In both of these cases, you can also clearly see that something is being assigned to a, right at the start of the line, without resorting to reading the whole thing, and figuring out if there are any game-changing function-calls happening on either the left or right side... or figuring out what both sides do, in general, to decide how many potential program-wide changes there might be.

If you need to include the whole type-check, it's still not that large.

a = (typeof a !== "undefined") ? a : 0;  // [parentheses are there for clarity]
Norguard
  • 24,349
  • 4
  • 38
  • 45
  • The ternary assignment (a = cond ? a : default) wastes an assignment when a passes the test. Writing cond || (a=default) only performs the assignment when a fails the test. This will matter more with ES5 (the new version of Javascript) and the arrival of properties, which can have assignment-style setters. – Phil H Sep 28 '12 at 09:47
  • @PhilH you keep saying that in your various comments, but I don't see it. `a = cond ? a : 0` is an assignment, sure, but so is `a = a || 0`. Where is the *extra* assignment you mention? – kojiro Sep 28 '12 at 12:01
  • 2
    The short version is not a = a || 0, but cond || a = 0. That behaves as if(cond) { a = 0}; you only perform the assignment *if the condition is true*. The other version will perform the assignment a = a whenever the condition is false, which is a waste. – Phil H Sep 28 '12 at 12:57
  • Side note, @PhilH, you can get the quote highlighting in comments by surrounding the text with backticks – Izkata Sep 28 '12 at 14:03
  • 1
    @PhilH -- you're talking about a "waste" in the realm of microseconds... This isn't added function-calls, et al. In fact, I could tell you that your way is a waste of a perfectly good test. `a` will be assigned itself, by default, unless the return value is falsey, and will then assign itself your given default. Likewise, your method is going to encourage devs all over to either "fix" an OR statement `(a === 0)`, or overload it `(a != 1) || (doStuff();doOtherStuff().id();a=16)`. Like people who already use ternary for branching execution... It doesn't end well for people maintaining your code. – Norguard Sep 28 '12 at 14:08
  • `a = a || 0` is about as readable as it gets. There are idioms developers using any language need to know. Using short-circuit evaluation to ensure that parameters have a default value is terse and easy on the eyes. Know your language and use it to enhance readability. This feature is one of the strengths of JavaScript. – Brent Faust Oct 09 '12 at 18:44
  • ...yes. And `a = a || 0;` is exactly what I offered. What should be avoided is something like `a || (doFunc().init() && a = 32 && (function () { if (x !== 42) { b = 12; }()))` – Norguard Oct 09 '12 at 18:48
9

is this legal, and cross browser valid?

Yes, it will work in all EcmaScript engines. However, it is very uncommon to (ab)use short-circuit-evaluation as an if-statement.

I mean, jslint says it has errors. Should I use it without concerns?

No, JsLint is right. It is unusual and confusing, at least to other developers. It looks too much like an OR-condition - yet is has no "body". And if you do assignments, the variable is expected to be on the beginning of the statement, not inside some expression.

I really like the second one because it is short, one line code

Then use

if (typeof a == 'undefined') a = 0;
Bergi
  • 513,640
  • 108
  • 821
  • 1,164
  • 2
    It's a big world. Using short circuit evaluation as an if-statement is not uncommon at all for shell script and perl programmers. I'm not sure I agree with the assumption that Javascript should look like C. – Random832 Sep 28 '12 at 12:27
2

You can use:

a = typeof(a) !== "undefined" ? a : 0;
Andy Hayden
  • 291,328
  • 80
  • 565
  • 500
Rahul
  • 511
  • 1
  • 7
  • 21
  • The ternary assignment (a = cond ? a : default) wastes an assignment when a passes the test. Writing cond || (a=default) only performs the assignment when a fails the test. This will matter more with ES5 (the new version of Javascript) and the arrival of properties, which can have assignment-style setters. – Phil H Sep 28 '12 at 09:48
1

Stylistically, setting default values like a || a=default is a common idiom on entry into a function, because javascript doesn't enforce the number of arguments.

Readability will be compromised if this construct is used in other circumstances, where you really mean if/else.

Performance used to vary between the different styles, but in a quick test today if/else and logical operators were the same speed, but ternary operation was slower.

Phil H
  • 18,593
  • 6
  • 62
  • 99
  • Why would you bother assigning to `a` on entry into the function at this point? That's the waste you were talking about. If `a` is a scalar, then you don't get a reference to the outer-scope's `a`, but rather the scalar value. If `a` is an object, then either you're wasting time assigning to `a`, because you're going to do your assignment through the return value (wasting assignment on the `a` argument), or you're going to do object-construction on `a`, which shouldn't be done there, because that's a horrible place for instantiation for code-maintainability. `var b = doIt(a||obj);` suffices. – Norguard Sep 28 '12 at 14:14
  • Because javascript doesn't guarantee your function arguments will be set in any way! If I construct `func(a,b,c)`, I can call it with just func(). Unless I set my default argument values on entry, I will have to check for undefined values anyway as soon as I use the arguments! In strongly typed languages you are guaranteed to have values, and references can guarantee that there is an object being passed, etc. Javascript guarantees none of these things. – Phil H Sep 28 '12 at 14:26
  • ...no, my question was, assuming `function add (a, b) { return a + b; }`, what's the difference between `var c = add(a||a=0, b||b=0);` and `var c = add(a||0, b||0);`. The answer is that for the purposes of `c` there is 100% no difference, whatsoever. – Norguard Sep 28 '12 at 14:47
  • Also, if it's the first time that `a` or `b` are being used in the program, and you intend to use them after the function call... ...or its the only place you do type-checking or validation on them in your program, that's a ***HORRIBLE*** place to do it. – Norguard Sep 28 '12 at 14:53
  • 1
    I meant more along the lines of `function add(a, b) { a || (a=0); b || (b=0); return a+b; }`. I agree, you wouldn't want to initialize global variables on entry to a function, but I'm not suggesting that at all. JS passes by value in the Java sense; values by value and objects by pointer value. So a function could test whether it has an object in an argument and construct a default object if it has not got one. Requiring the caller to do that expects too much of the caller, and implies you never want to have optional arguments. – Phil H Sep 28 '12 at 14:57
1

May I ask why you prefer one line of code?

As a human, I prefer readable code. My machine prefers short and fast code (easy to load and execute).

Today minifiers like UglifyJS know how to shorten code, so you can have both and you don't need to worry about this level of detail. I gave your code to UglifyJS and here is the output:

typeof a=="undefined"&&(a=0)

You can try it out here:

http://marijnhaverbeke.nl/uglifyjs

[Update] My personal preference (again with readability in mind) is to use if when there are choices, and || for fallbacks. Your specific example seems to be a fallback (if a doesn't exist or is undefined, then assign to a the value 0), so I'd use ||. As I said in a comment, (var a=0) would make more sense to me in cases where the variable a hasn't been declared yet (I don't know your context).

Christophe
  • 24,147
  • 23
  • 84
  • 130
1

I think that it is terrible to use constructions like this. It works but code not readable. If you want to write one-line conditions you can start using CoffeeScript and write:

a = 0 if (typeof a == 'undefined');

In your case, when you have one variable in condition and assignment, use one-line javascript ternary operator:

a = (typeof a == 'undefined') ? 0 : a;
Alexandr Subbotin
  • 1,614
  • 2
  • 16
  • 16
1
(typeof a != 'undefined') || (a = 0)

is an expression. But one of its subexpressions

(a = 0)

is an assignment. So essentially, this is an expression with a side effect. Statements (especially those that are expessions), that have side effects are typically one of the first things you learn not to do in an introductory coding class. So why do it simply because it only takes one line?

hjess
  • 11
  • 2
0

I hated "|| instead of IF" for years.

I finally got used to it and now I love it.

I also like using && in the same way.

I've found it is much easier to read others terse code if you adopt terse practices yourself.

Totally understand where others are coming from though. Been there myself.

Matthew Baker
  • 1
0

Moreover I suppose it would impact performances because

(typeof a != 'undefined') || (a = 0)

is composed of two test (even if we don't care about the second one).

Whereas

if (typeof a == 'undefined') {
    a = 0;
}

contains only one test

Angelblade
  • 61
  • 6