Running query on collection items in nodejs

Question

So I am stumped, and just cannot seem to grasp what is going on. I have a function that takes an array of orders.

var collection = [{ordernumber: 1, href: 'FileDetails.aspx?FileId=1234'}, 
                  {ordernumber: 2, href: 'FileDetails.aspx?FileId=1478'}];
var OrdersListToImport = [];

function loopOrders(collection, callback) {
  for(var i = 0; i < collection.length; i++) {
    console.log('processing order #: ' + collection[i].ordernumber);
    var answersReturned = 0;
    db.needsImported(collection[i].ordernumber, function(answer) {
      if (answer) {
        OrdersListToImport.push(collection[i]);
        //console.log(collection[i]);
      }
      if (++answersReturned == collection.length) {
        callback();
      }

    });
  }

}

NeedsImported function as follows:

needsImported: function(ordernumber, callback) {
    pool.query('Select controlnumber From orders Where ordernumber = ?', [ordernumber], function(err, result) {
      if (!err) {
        if (result.length == 0) {
          callback(true);
        }
        else {
          callback(false);
        }
      }
    });
  }

When inside the callback function of db.needsImported, collection[i] becomes undefined. It was driving me mad, so i made a little sample file to see if there was a reason I couldn't access an argument from inside a callback function. It works as expected, only pushing even numbers. here is the sample:

var nums = [];
var collection = [1,2,3,4,5,6,7,8,9,10];

displayValue(collection, function() {

});
console.log(nums);

function displayValue(col, callback) {
  for(var i = 0; i < col.length; i++) {
    sleep(col[i] * 500, function() {
      console.log('Count: ' + col[i]);
      if (col[i] % 2 == 0) {
        nums.push(col[i]);
      }
    });
    callback();
  }
}

function sleep(time, callback) {
  var stop = new Date().getTime();
  while(new Date().getTime() < stop + time) {
      ;
  }
  callback();
}

I hope someone can help me to understand what I am doing wrong.

Answer 1

You might see more clearly what the problem is if you were to console.log(i) in the spot where you expect collection[i] to not be undefined.

What's going on in your for loop is you're iterating through your array and firing off multiple asynchronous tasks. When the loop is finished, the value of i is 2. i is a closed variable accessible to your callbacks, but remember the asynchronous timing here, your needsImported callbacks are all getting invoked after your for loop has completed, so naturally, collection[2] is undefined. To fix it you could just get rid of the index and use Array.forEach instead:

function loopOrders(collection, callback) {
    var answersReturned = 0;
    collection.forEach(function(item, index) {
        needsImported(item.ordernumber, function(answer) {
            console.log('index:', index);
            if (answer) {
                OrdersListToImport.push(item);
            }
            if (++answersReturned == collection.length) {
                callback();
            }
        });
    });
}

Note also that if you do need to keep track of the index in your callback, you can receive it as the second parameter in your forEach callback as I've done above.

One more note :) There's a significant flaw in this code:

needsImported: function(ordernumber, callback) {
    pool.query('Select controlnumber From orders Where ordernumber = ?', [ordernumber], function(err, result) {
      if (!err) {
        if (result.length == 0) {
          callback(true);
        }
        else {
          callback(false);
        }
      }
    });
  }

If an err does occur, you're ignoring it, but worse than that, you're not invoking the callback. Confusing bugs will emerge from this. It's common practice in node to serve an err var as the first argument of a callback, so in a success scenario you would do this callback(null, true).