4 ms·

Can someone explain how this is potentially dangerous? if (!cachedReturnValues[key]) { cachedReturnValues[key] = value; }

by ronaldj 14y ago

Leszek 14y ago
The condition will be true in any of the below cases: cachedReturnValues[key] === false cachedReturnValues[key] === 0 cachedReturnValues[key] === "" cachedReturnValues[key] === null cachedReturnValues[key] === undefined (this is presumably the only desired behaviour) cachedReturnValues[key] === NaN
- dagw 14y ago
  But surely all that happens in these cases is that the function gets re-evaluated, leading to marginally worse performance in a few cases (for example functions that return 0 a lot). Unless your function has side effects and it's vital that it is only run once for each argument I can't think of a scenario where that code is dangerous.
  - pretoriusB 14y ago
    >But surely all that happens in these cases is that the function gets re-evaluated (...) Unless your function has side effects and it's vital that it is only run once for each argument I can't think of a scenario where that code is dangerous. Then this kind of code would have blown in your face. Besides the function being re-evaluated, the hash gets a NEW value "value" for that key. Nobody ensures that "value" is the same as the one it was already there. E.g var cachedReturnValues = {key: 0}; var value = 50; if (!cachedReturnValues[key]) { cachedReturnValues[key] = value; } So, instead of setting the value when the key is not already in the hash, now we also CHANGE it in all these false-positive cases. Furthermore, even if (a) the function had no side-effects, and (b) by some magic the value was the same with what was already in the hash, e.g if your wrong "merely re-evaluation" assumption was true, who would ensure that the overall block has no side-effects? if (!cachedReturnValues[key]) { cachedReturnValues[key] = value; doSomethingElse(); } Just because we see it written in a way that doesn't do much harm now, doesn't mean it's not extremely dangerous to blow up in the future, e.g. when some naive programmer adds another statement to the block.

Can someone explain how this is potentially dangerous? if (!cachedReturnValues[key]) { cachedReturnValues[key] = value; }

by ronaldj 14y ago

Can someone explain how this is potentially dangerous? if (!cachedReturnValues[key]) { cachedReturnValues[key] = value; }

Leszek 14y ago

The condition will be true in any of the below cases: cachedReturnValues[key] === false cachedReturnValues[key] === 0 cachedReturnValues[key] === "" cachedReturnValues[key] === null cachedReturnValues[key] === undefined (this is presumably the only desired behaviour) cachedReturnValues[key] === NaN

dagw 14y ago
But surely all that happens in these cases is that the function gets re-evaluated, leading to marginally worse performance in a few cases (for example functions that return 0 a lot). Unless your function has side effects and it's vital that it is only run once for each argument I can't think of a scenario where that code is dangerous.
- pretoriusB 14y ago
  >But surely all that happens in these cases is that the function gets re-evaluated (...) Unless your function has side effects and it's vital that it is only run once for each argument I can't think of a scenario where that code is dangerous. Then this kind of code would have blown in your face. Besides the function being re-evaluated, the hash gets a NEW value "value" for that key. Nobody ensures that "value" is the same as the one it was already there. E.g var cachedReturnValues = {key: 0}; var value = 50; if (!cachedReturnValues[key]) { cachedReturnValues[key] = value; } So, instead of setting the value when the key is not already in the hash, now we also CHANGE it in all these false-positive cases. Furthermore, even if (a) the function had no side-effects, and (b) by some magic the value was the same with what was already in the hash, e.g if your wrong "merely re-evaluation" assumption was true, who would ensure that the overall block has no side-effects? if (!cachedReturnValues[key]) { cachedReturnValues[key] = value; doSomethingElse(); } Just because we see it written in a way that doesn't do much harm now, doesn't mean it's not extremely dangerous to blow up in the future, e.g. when some naive programmer adds another statement to the block.