Why does JSHint dislike ternaries for method calls on objects?

Question

JSHint give the following error:

Expected an assignment or function call and instead saw an expression.

For the following line of code:

(aFunctionOrNull) ? aFunctionOrNull() : someObject.someMethod();

It highlights the final ) on someMethod so I assume the error is there. The code works and JSHint doesn't have a problem when I change it to if () {} else {} syntax. I don't mind the longer syntax but I'd like to learn why JSHint says this and if this is a bad practice.

The biggest piece of confusion may come from the terminology. Is someObject.someMethod() not a function call?

Answer 1

Well, in general it's considered bad practice to call a function using the ternary operator(s), without assigning the return value (which is what you seem to be doing).
Also, it could be worth checking what JSHint has to say about the following code:

(aFunctionOrNull || someObject.someMethod)();

If aFunctionOrNull is undefined (or null, or falsy), the logical-or-bit will cause the expression to evaluate to someObject.someMethod, and the resulting value of that is invoked (a reference to a function object, hopefully). This gives you the opportunity to write your code more "fail-safe" without the bulk of a nested ternary:

(aFunctionOrNull || someObject.someMethod || function(){})();

The grouped expression is now bound to evaluate to a truthy value, so no errors are thrown there.
To avoid JSHint nagging about your not doing anything with the return value, either assign it to a variable (which I don't really like doing), or add a little operator to the mix:

~(aFunctionOrNull || someObject.someMethod || function(){})();//bitwise not
!(aFunctionOrNull || someObject.someMethod || function(){})();//logical not, doesn't really matter which one

On your last question: someObject.someMethod is indeed a function call. More specifically, it's a call to a function object in the someObject's context.
For those who don't know this: JS functions are objects, and the called context is either explicitly set using the bind method (defined on the Function.prototype) or ad-hoc:

var referenceToMethod = someObject.someMethod;
referenceToMethod();//<-- inside the function objects, this now points to the global object

An easy way to think of it is that JS functions just float around aimlessly in memory/space/time, until they are called via a reference, the context of that reference is then passed to the function object, to determine what object it'll interact with. This is, sadly, the global object by default, or null in strict mode.

Answer 2

JSHint says about expressions, or expr:

This option suppresses warnings about the use of expressions where normally you would expect to see assignments or function calls. Most of the time, such code is a typo. However, it is not forbidden by the spec and that's why this warning is optional.

While JSLint says:

An expression statement is expected to be an assignment or a function/method call or delete. All other expression statements are considered to be errors.

AFAIK, there's no problem in doing what you're doing only that it will issue a warning because it would expect you to use an if..else statement, but you can turn this off in JSHint with:

/*jshint expr:true */

Answer 3

There error is because a ternary is an expression. You could use it to set a variable:

var result = a ? b : c;

Notice that the ternary evaluates to either b or c. It's an expression.

That said, the warning (I believe) comes from the notion that ternaries suffer poorer readability than an if...else block. The code above can be rewritten

var result;
if (a) {
    result = b;
} else {
    result = c;
}

Which is easier to read than a ternary. JSHint does as much to promote readable code as it does valid code. If you're comfortable including these expressions in your code, go ahead and disable the warnings for expressions. (It's what I would do.)