-1

I don't understand the following if-then-else clause, which I found in a piece of code I'm working on.

if (prefstocking && prefstocking >0) {
    ...
} else {
    ...
}

Why does the variable prefstocking appear on both sides of the logical operator &&? I thought using the logical operator && meant using both of them, like this: if (x && y = 1) makes sense to me, meaning "if x equals 1 and y equals 1", but what is the meaning of using the same value twice?

Radiodef
  • 37,180
  • 14
  • 90
  • 125
kailyn
  • 9
  • 1
    `prefstocking && prefstocking > 0` means `(prefstocking) && (prefstocking > 0)`. So, it is checking to make sure that prefstocking isn't falsey, and that it is greater than 0. seems reasonable – wolffer-east May 08 '15 at 19:54
  • @wolffer-east but `([false|0|undefined|null|''] > 0)` is false; why bother checking for falsiness at all? – Evan Davis May 08 '15 at 20:00
  • @Mathletics because I have never actually tested `([false|0|undefined|null|''] > 0)`. Now it doesn't seem reasonable at all :). The logical separation of the conditions is still important to note, the validity of having both notwithstanding – wolffer-east May 08 '15 at 20:03
  • @wolffer-east that's my point; people cruft-up their code with with this stuff because they don't think logically about what's happening. – Evan Davis May 08 '15 at 20:07
  • @wolffer-east _The logical separation of the conditions is still important to note_, how do you mean? Vs, say, `(prefstocking && prefstocking) > 0`? Actually in rereading OPs post, I've answered my own question and I agree. – Evan Davis May 08 '15 at 20:09
  • @Mathletics - It may also be for consistency . . . while it may not be needed in this case, it is needed in cases like `prefstocking && prefstocking.foo > 0`. Doing it the same way every time could help people avoid forgetting to do it when it's actually needed (not saying that it is a definitively good idea, but I would understand the logic behind it behavior). – talemyn May 08 '15 at 20:12
  • 1
    Other people here seem to be ignoring the fact that if the first test fails then the interpreter doesn't even need to check the second condition---and it probably won't. Even though extra conditions may be mathematically redundant, because of the order of computation, it may still be valuable to include redundant easy checks first to improve efficiency. – RudePeopleStepOff May 08 '15 at 20:19
  • Why did people vote this question down? It was valid and useful. – RudePeopleStepOff May 08 '15 at 22:03
  • IMO, depending on what `prefstocking` is supposes to contain, this kind of test can actually make sense for readability reasons, even if it is redundant. Suppose it is a variable that can be either `null` or any integer. The `if clause` would actually reflect the nature of that variable better and might make the code easier to understand. – basilikum May 08 '15 at 22:08

3 Answers3

2

The first part if (prefstocking && ...) checks the var prefstocking for false, null, undefined, 0, NaN, and the empty string. These are all called "falsy" values.

If prefstocking is "falsy" then it isn't greater than zero and doesn't need to check that.

Another answer goes into some detail about truthy v. falsy in javascript.

In this case it makes no difference if the test is if (prefstocking > 0) because that will always evaluate to the same result as the original, but the principal is often useful, especially to avoid dereferencing a null or undefined object.

var obj1 = someFunction('stuff', 9); // assume it returns an object
var obj2 = getNullObj(); // assume it always returns null

// this is OK if an object is always returned from the someFunction(...) call
if (obj1.hasData()) { }
// this causes an error when trying to call the .hasData() method on a null or undefined object
if (obj2.hasData()) { }

But, because the logical and && and the or || operators short-circuit, testing like this is safe:

if (obj2 && obj2.hasData()) { }

If the first part is false (falsy) it won't try to evaluate the second part because the logical truth is already know - the whole statement is false if the first part of an and is false. This means .hasData() will never get called if obj2 is null or undefined.

If an object is defined but does not have a .hasData() function then this will still cause an error. Defending against that could look like

if (obj2 && obj2.hasData && obj2.hasData()) { }
// ...or...
if (obj2 && typeof obj2.hasData === 'function' && obj2.hasData()) { }

Short-circuiting allows you to check and avoid failure cases, but checking every possible failure could make your code unreadable and perform poorly; use your judgment.

Community
  • 1
  • 1
Stephen P
  • 14,422
  • 2
  • 43
  • 67
2

Written in plain English, this test reads:

if prefstocking is truthy and its value is greater than 0

however, because most values are truthy, the former check is unnecessary. Any case which fails the first condition would also fail the second. I see a lot of developers write these kind of checks to be extra-sure, but it tells me that they simply aren't thinking about what they're doing.

Evan Davis
  • 35,493
  • 6
  • 50
  • 57
  • It is checking for more than whether or not the variable exists (is defined), as `""`, `null` and `0` "exist". It looks more like they are checking whether or not the value is `0` and then going on to check if it's non-negative (though that is speculation). – Rob M. May 08 '15 at 20:01
  • @RobM. but in a single expression, what's the point? If the conditions were nested, it might be reasonable. But in a single expression, `a > 0` tells you everything `a && a > 0` would tell you. – Evan Davis May 08 '15 at 20:02
  • 1
    I agree completely, it is a pointless check in this particular instance; I was just pointing out that the condition is checking for more than whether or not the variable is defined. – Rob M. May 08 '15 at 20:04
  • If the first condition is false, then the next condition will not even be checked. Because of this, conceivably it's a matter of computational efficiency sometimes to do something of this form. – RudePeopleStepOff May 08 '15 at 20:15
  • 1
    @RudePeopleStepOff that would be easy to benchmark but sounds like a ridiculous micro-optimization. – Evan Davis May 08 '15 at 20:25
  • That's not relevant at all. – RudePeopleStepOff May 08 '15 at 20:26
  • @RudePeopleStepOff huh? – Evan Davis May 08 '15 at 20:29
  • 1
    The _size_ of the optimization in this instance isn't relevant. Aren't we trying to write answers that may help more than just this particular person in his very specific situation? In a different setting with the same basic structure, the advantages could be huge. – RudePeopleStepOff May 08 '15 at 20:30
-1

Others are correct in pointing out that the way to read this is (prefstocking) && (prefstocking > 0). The first condition checks whether prefstocking is truthy. The second condition makes sure it's greater than 0. Now, as to why bother doing that? Here I disagree with the other answers.

There are situations in programming where we might use redundant conditions in an if then clause because of efficiency. In this situation, mathematically speaking the first condition is redundant. That is, if the second condition is true, then the first condition is also true. However, order matters. An if an interpreter checks the first condition and finds it false, followed by an && (and), then it doesn't need to test further. And it probably won't test the second condition (see comments below: according to ECMAScript standard, it definitely won't test the second condition). This could be useful if it is less computationally expensive to check the first condition, such as first ruling out null cases. The specifics of whether it's actually more efficient are hard to quantify with JavaScript because the internals are often not specified and each JS interpreter works in its own way.

Also, an expression of the form if (x && y == 1) would be interpreted as "if x is truthy and if y equals 1". You have misunderstood the order of operations. Both sides of the && make separate conditions. They don't combine into one condition like the might in English. This expression certainly does not mean "if x and y equal 1". Make sure you have understood that.

RudePeopleStepOff
  • 39
  • 8
  • 1
    _"it probably won't test the second condition"_ — it is _guaranteed_ not to test the second condition if the first is false, as both `&&` and `||` use [short-circuit](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_Operators#Short-Circuit_Evaluation) evaluation rules. – Stephen P May 08 '15 at 21:36
  • @StephenP Good. Then what I said is true. But how do you know what JavaScript interpreters actually do internally? – RudePeopleStepOff May 08 '15 at 21:37
  • 3
    The ECMAScript specification defines the language to do this, so any conforming interpreter will do it. You could test any given javascript engine by creating a suite that does things such as `if ( callFalse() and callTrue() ) { console.log("test #1 failed"); } else { console.log("test #1 passed"); }` -- and also have callTrue() and callFalse() write to the console to show they were (or were NOT) called. – Stephen P May 08 '15 at 21:56
  • @StephenP Good. I sincerely appreciate you adding more information. I hope you won't hold it against me that I used conservative language. Now that you have commented, future readers will have a more complete understanding. I voted your comments up. – RudePeopleStepOff May 08 '15 at 21:57
  • @StephenP answer is very good and offers more details on some of the topics I have mentioned here. – RudePeopleStepOff May 08 '15 at 23:46