Is that js expression safe: if( !x || doSomething( x[prop], y[prop] ) === false )

Question

From a bug report, I think that the following expression might throw an exception if x is null:

if ( !x || doSomething( x[prop], y[prop] ) === false )

The exception is:

Cannot read property 'prop' of null

... as if the right side of the || is evaluated even if the left side is true. The javascript reference seems to indicate that that should not happen, but I'm not sure. I've tested that just writing x = null does not (always) crash, but is it guaranteed on every JS engine ?

EDIT:

Same question about

if( x && foo( x[prop] ) === true && bar() === false )

One way to put it is, does :

if( a && b && c )

... evaluates b or c if a === false ? The doc is not clear about that case, only for "a && ( expr1 && expr2 )", not "a && expr1 && expr2"

Full code snippet

var x = null;
var y = {
  "p1": "p1",
  "p2": "p2"
};

function f() {
  return true;
}

for (var propName in y) {

  if (x && f(y[propName]) === true && f(y[propName]) === false) {
    doSomething(x[propName], y[propName]);
  } else if (!x || f(x[propName], y[propName]) === false) {
    console.log(y[propName]);
  }
}

EDIT2: for completeness, the real (minimized) code that run in the browser

  function a(c, b, e, f) {

        for (var d in b) {
          if (c && _.isObject(b[d]) === true && _.isArray(b[d]) === false) {
            a(c[d], b[d], e, d + ".")
          } else {
            if (!c || _.isEqual(c[d], b[d]) === false) {
              e.push({
                name: f + d,
                value: b[d]
              })
            }
          }
        }
        return e
      }

can you provide jsfiddle with sample? are you sure that x is null? are you sure that exception in this line? — Grundy, Dec 18 '15 at 14:55
Use `if (x && y && x.hasOwnProperty(prop) && y.hasOwnProperty(prop) && doSomething(x[prop], y[prop]) === false)` — Tushar, Dec 18 '15 at 14:55
@Tushar `foo.hasOwnProperty('bar')` is not the same as "can I [[GET]] _bar_ from `foo`?" Instead, `('bar' in foo)` is more reliable — Paul S., Dec 18 '15 at 14:57
Documentation says: ["The rules of logic guarantee that these evaluations are always correct. Note that the anything part of the above expressions is not evaluated ..."](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_Operators#Short-Circuit_Evaluation) — Teemu, Dec 18 '15 at 15:01
We don't really know if OP wants to check properties on the prototype chain or just the object, so it could be any. — MinusFour, Dec 18 '15 at 15:02
I'm sure that y is not null because the code is in a loop: for( var prop in y) — Ilya, Dec 18 '15 at 15:05
I have a feeling the error isn't here, unless `y` has a property named 'prop`. — MinusFour, Dec 18 '15 at 15:09

deceze · Answer 1 · 2015-12-18T14:58:06.597

3

The Javascript || operator is short-circuiting. The right-hand side will not evaluate if the left-hand side is true. That's a fundamental property of the operator and should be equally implemented across all engines.

Therefore, the right-hand side will only evaluate if x is truthy, and all truthy values in Javascript should be subscriptable without error.

Having said that, y is completely unknown in this example and might throw an error.

edited Dec 18 '15 at 14:58

answered Dec 18 '15 at 14:55

deceze

510,633
85
743
889

1

> y is completely unknown in this example and might throw an error @deceze Yes "y" somewhat obscures the question, I should haved removed it as it's always not null – Ilya Dec 18 '15 at 16:18

score 2 · Accepted Answer · answered Dec 18 '15 at 17:07

2

"Is it guaranteed on every JS engine?"

We can't actually know that for sure, but the standard defines, how these operators should be implemented.

Logical OR:

Let lref be the result of evaluating LogicalORExpression.

Let lval be GetValue(lref).

If ToBoolean(lval) is true, return lval.

Let rref be the result of evaluating LogicalANDExpression.

Return GetValue(rref).

http://es5.github.io/#x11.11

Item 3 doesn't leave any room to doubts, lval is returned immediately if lref can be evaluated to truthy, and rref will never be evaluated.

answered Dec 18 '15 at 17:07

Teemu

22,918
7
53
106

so there's no doubt about "(a || b===c)", b===c should not be evaluated. I'm still not sure about ( a && b && c ) with a===false, are b or c evaluated ? – Ilya Dec 18 '15 at 17:33
@Ilya If you'd put the parents to the expression to show the correct execution order (`( (a && b) && c )`), then you'd see. If `a===false`, `b` or `c` can be even not defined at all. – Teemu Dec 18 '15 at 17:46
OK, so the exception in that code is still a mystery to me. It happened, seemingly randomly, inside that function, and testing the x == null case beforehand seemed to fix the problem. Very strange. – Ilya Dec 18 '15 at 18:14
@Ilya Based on what you've posted, it's impossible to even speculate a reason for random errors. Maybe you should ask a new question about your real problem, include the code defining `x` and the "loop", and include also the source of "bug report". – Teemu Dec 18 '15 at 18:52
I've added the code with the loop in the question. The bug report is a log of browser (Chrome) exceptions including the stack trace. So I'm sure that the exception is in the loop, in the if() or the else(), from one of the [propName] expressions, because x is null. So the real problem is fully contained in the loop: according to my understanding of the JS standard, a null x shouldn't trigger an exception because the x[propName] should not be evaluated. – Ilya Dec 18 '15 at 19:33
@Ilya Like I said, please ask a __new__ question. Asking about JS behavior and about errors randomly occuring in some code are totally diffrerent subjects. – Teemu Dec 18 '15 at 19:36

score 0 · Answer 3 · answered Dec 18 '15 at 14:58

if (typeof y != 'undefined' && typeof x != 'undefined' && x !== null && y !== null) {
   if (doSomething( x[prop], y[prop] ) === false) {
       //do stuff
   }
}

do the safety check before. this should be working

but note:

if your prop Attribute does not exist, this will return an error too!

greetings

Is that js expression safe: if( !x || doSomething( x[prop], y[prop] ) === false )

3 Answers3

Linked