Skip to content

Instantly share code, notes, and snippets.

@paul-jean
Last active January 18, 2016 02:31
Show Gist options
  • Select an option

  • Save paul-jean/9b01ca63dd1636e602bc to your computer and use it in GitHub Desktop.

Select an option

Save paul-jean/9b01ca63dd1636e602bc to your computer and use it in GitHub Desktop.
using the presence of an object value as truthy is a BAD idea

This is an example of not being precise enough when checking for a value in an object. I inadvertently caused this bug during a live coding exercise in a job interview, and spent a couple minutes debugging before I found it.

The exercise was to code up the Fibonnaci sequence, and I was doing a variation where I memoized previously computed values of the sequence (dynamic programming). I had set up an object to hold values of the sequence as key-value pairs, where the key is n and the value is f(n).

I called the object fibMemo, and initialized it with the first two entries in the Fibonnaci sequence f(0) = 0 and f(1) = 1:

var fibMemo = {0:0, 1:1};

Inside the function I checked for previously memoized values of f(n) using a conditional of the form

if (fibMemo[n]) {
  // use the memoized value
} else {
  // compute f(n) and memoize it
}

For almost all cases this works fine, since if fibMemo[n] hasn't been memoized for some n (say n=5), then fibMemo[n] ==> undefined, which is falsey. Then the else condition will evaluate and memoize the value for future calls. Exactly what I want.

HOWEVER, in the case of n = 0, while fibMemo[0] IS memoized, fibMemo[0] ==> 0. In this case even though the memoized value exists, the value returned is zero, which is falsey! So while fibMemo[n] is always falsey if n is NOT memoized, it is NOT always truthy if it IS memoized.

My code assumes that fibMemo[n] is truthy if memoized and falsey if not memoized, for any value of n >= 0. Since the n = 0 case never returns a value, the recursion proceeds unbounded into negative values of n, and overflows the call stack. (This is a remaining problem in the code: there's no check to make sure n >= 0.)

The better way to check for the memoized values is to explicitly check for the type of the value in the memoized object:

if (typeof fibMemo[n] === 'number') {
  // memoized
} else {
  // not memoized
}

Now the conditional is always truthy if the value IS memoized, and always falsey if the value IS NOT memoized.

var fibMemo = {0:0, 1:1};
var fib = function(n) {
if (fibMemo[n]) { // <--- fibMemo[0] IS memoized, but the value is 0, which is falsey!
console.log('using fibMemo: n = ' + n)
return fibMemo[n];
}
else {
fibMemo[n] = fib(n-2) + fib(n-1);
return fibMemo[n];
}
};
console.log(fib(5));

Running the buggy code exceeds the recursion limit, because even though fibMemo[0] is defined, it returns 0, which is falsey!

[rule146@rule146: js-undefined]$ node buggy.js
using fibMemo: n = 1
/Users/rule146/code/js-undefined/buggy.js:4
  if (fibMemo[n]) {
             ^

RangeError: Maximum call stack size exceeded
    at fib (/Users/rule146/code/js-undefined/buggy.js:4:14)
    at fib (/Users/rule146/code/js-undefined/buggy.js:9:18)
    at fib (/Users/rule146/code/js-undefined/buggy.js:9:18)
    at fib (/Users/rule146/code/js-undefined/buggy.js:9:18)
    at fib (/Users/rule146/code/js-undefined/buggy.js:9:18)
    at fib (/Users/rule146/code/js-undefined/buggy.js:9:18)
    at fib (/Users/rule146/code/js-undefined/buggy.js:9:18)
    at fib (/Users/rule146/code/js-undefined/buggy.js:9:18)
    at fib (/Users/rule146/code/js-undefined/buggy.js:9:18)
    at fib (/Users/rule146/code/js-undefined/buggy.js:9:18)
var fibMemo = {0:0, 1:1};
var fib = function(n) {
if (typeof fibMemo[n] === 'number') { // <---- better to check for presence of a value this way
console.log('using fibMemo: n = ' + n)
return fibMemo[n];
}
else {
fibMemo[n] = fib(n-2) + fib(n-1);
return fibMemo[n];
}
};
console.log(fib(5));

Doing a more precise check for the presence of a value in the fibMemo object corrects the problem:

[rule146@rule146: js-undefined]$ node better.js
using fibMemo: n = 1
using fibMemo: n = 0
using fibMemo: n = 1
using fibMemo: n = 2
using fibMemo: n = 3
5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment