CODEWITHSUNDEEP
javascript
Tamás Pap
Tamás Pap

Reputation: 18293

Javascript: || instead of IF statement - is this legal and cross browser valid?

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?

Upvotes: 33

Views: 6933

Answers (10)

hjess
hjess

Reputation: 11

(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?

Upvotes: 1

Alexandr Subbotin
Alexandr Subbotin

Reputation: 1734

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;

Upvotes: 1

Angelblade
Angelblade

Reputation: 71

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

Upvotes: 0

Christophe
Christophe

Reputation: 28154

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).

Upvotes: 1

Matthew Baker
Matthew Baker

Reputation: 1

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.

Upvotes: 0

Bergi
Bergi

Reputation: 664650

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;

Upvotes: 9

Phil H
Phil H

Reputation: 20151

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.

Upvotes: 1

Rahul
Rahul

Reputation: 601

You can use:

a = typeof(a) !== "undefined" ? a : 0;

Upvotes: 2

Tomasz Nurkiewicz
Tomasz Nurkiewicz

Reputation: 340783

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.

Upvotes: 76

LetterEh
LetterEh

Reputation: 26696

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]

Upvotes: 31

Related Questions