JavaScript coding technique or bad code?

Question

While debugging javascript written by someone else, I came across some code that I've not seen before. Here's a sample:

function doSomething() {
    //doing something here...
}

function doItNow() {
    //other logic...
    doSomething && doSomething();    // <=== What's this?
}

Is the purpose of the 2nd line in function doItNow() to check if doSomething exists and then call it? Like so:

function doItNow() {
    //other logic...
    if (doSomething) {
        doSomething();
    }
}

JSLint does not like it and I'd rather not have bad code in my app. Any insights?

Equi

I think it's pretty clever although it does hurt the readability of the code. I wouldn't call this "bad code" more "code with poor readability" – Sherwin

Depending on language, this can be considered a standard idiom. – Armhole

Answer 1

It's a 'shorthand' indeed. The right side is only executed when the left side passes as if() statement.

Google Closure Compiler and other minifiers take advantage of this; if your input is if(a) a(), it will result in a&&a()

---

You could do the same with ||, for example:

if( !a ){
  alert('Not a');
}

can be written as

a || alert('Not a');

Uphold

Answer 2

Yes, your two examples are "equivalent", the && operator performs short-circuit evaluation.

If the first operand expression yields a falsey value (such as null, undefined, 0, NaN, an empty string, and of course false), the second operand expression will not be evaluated, and if the value is truthy, the function call will be made.

But if doSomething hasn't been declared, your both examples will fail.

If an identifier that's not declared, is referenced on code, you will get a ReferenceError exception, e.g.:

function foo() {
  undeclared && undeclared();
}

try {
  foo(); 
} catch (e) {
  alert(e);  // ReferenceError!
}

If you want to:

1. Make sure the identifier exist, and
2. Make sure it is callable

You can:

if (typeof doSomething == 'function') {
  doSomething();
}

The typeof operator can be safely used on identifiers that don't exist, additionally, by checking that doSomething is a function, you make sure that you will be able to invoke it.

Linguist

@Tim, the accepted answer is the most direct answer given the context of my question. Also, most JS code is minified in production and most minifiers change the "if" condition to shorthand. So IMHO, it's better to at least understand the shorthand thoroughly since that is what you will see more often than not. – Equi

@Silkster: The bit about doSomething not being declared is crucial information. If doSomething is not declared (say you deleted the function declaration), you will get an error. – Sorghum

Answer 3

Calling functions (or assignments, etc) in comparisons is generally a bad idea. People don't usually expect comparisons to have side effects. This case is simple enough that it might be justifiable, but if someone doesn't understand the convention they might have to ask on StackOverflow ;)

Chatterer