Loops, Scope, and Asynchronous Code in JavaScript

Once upon a time, there was a web page. It was a tiny web page, one of many like itself, but even the tiniest web page is special and so it was in fact quite special.

It was quite special because it had a spectacular bug in it that caused me to rip my hair out for the course of several hours as a novice programmer and curse the day I decided against becoming a baker.

The Page

The page was simple and delicate, like a beautiful little bird. I wanted to send out AJAX requests for 5 different vehicles and list their locations.

What better way than a loop?

I don't have the code anymore, but it went something like this:

for (var i = 0; i < vehicles.length; i++) {
  $.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
    vehicles[i].location = locationData.coords;
  });
}

Something like that.

If you can't spot the bug, don't worry.

After running your code, you will encounter an error of TypeError: vehicles[i] is undefined five times. This is not particularly useful. To try and understand it, I checked through a few possibilities while debugging.

Debugging

Possibility 1: Is the request wrong?

My first thought was "Oh, I must be requesting the wrong resource". The net inspector showed that I was requesting different vehicles each time and that five requests were firing off. Damn.

Conclusion: No. Absolutely not.

Possibility 2: Is my response wrong?

I was relieved. Something was probably wrong in the response and the locationData was probably null; probably querying the wrong vehicle, or something.

This didn't make any sense, but I figured it was a possibility that the error message was wrong.

I inspected the response. No, no this was clearly on my end; the vehicles were where we expected them to be in the response.

Well, one was slightly off route, but not my problem.

Conclusion: No, this is probably something I'm doing wrong.

Possibility 3: Am I targeting the wrong vehicle?

I scratched my head, because I didn't understand how to use the debugger quite well back then. If I could, I would have realized the issue quite sooner.

I decided to start console logging.

for (var i = 0; i < vehicles.length; i++) {
  console.log(vehicles[i]);
  console.log(vehicles[i].id);
  console.log("/vehicles/location" + vehicles[i].id);
  
  $.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
    console.log(locationData);
    console.log(vehicles[i]);
    console.log(vehicles[i].id);
    vehicles[i].location = locationData.coords;
  });
}

I was a little over-zealous with the logging at that point.

Everything was just fine until console.log(vehicles[i]); when I got an undefined each time.

I changed it to log one final thing:

for (var i = 0; i < vehicles.length; i++) {
  $.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
    console.log(i);
    vehicles[i].location = locationData.coords;
  });
}

And I got 5 printed out 5 times.

Well that's not good. The array stopped at vehicles[4] and why was it 5 anyways?

Conclusion: Maybe I made a baker's quartet?

Real conclusion: i isn't what I expect it to be.

Understanding the bug

At this point, I did not fully understand scopes or asynchronous code.

In our loop, we are running asynchronous code. This means that code may not run in the order that it is syntactically written. I expected that.

The callback function in the promise returned from our $.get call is to be run when the AJAX request is finished, which can happen at any part after it was called. The parameter to $.get, "/vehicles/location" + vehicles[i].id is run synchronously (as it should), meaning that the URL is properly formatted.

At the point when the $.get function is called, i is the expected value of whatever index we are in while traversing the array.

When starting the AJAX call, the loop moves forward and i updates accordingly. By the time the callback is run, the entire loop is complete -- our variable i, is 5 inside that callback because the loop has completed and incremented it accordingly.

You can view the loop, effectively, as unraveling to this:

var i = 0;
var vehicles = [{id: "some-id-1"}, {id: "some-id-2"}, {id: "some-id-3"}, {id: "some-id-4"}, {id: "some-id-5"}];

// i = 0;

$.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
  vehicles[i].location = locationData.coords;
});

i = i + 1; 

// i = 1;

$.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
  vehicles[i].location = locationData.coords;
});

i = i + 1; 

// i = 2;

$.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
  vehicles[i].location = locationData.coords;
});

i = i + 1; 

// i = 3;

$.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
  vehicles[i].location = locationData.coords;
});

i = i + 1; 

// i = 4;

$.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
  vehicles[i].location = locationData.coords;
});


i = i + 1; 
// i = 5;
// loop stops here


// move on with your lives

By the time any callback runs, i = 5 makes a lot more sense.

Fixing the bug

Admittedly, I only vaguely understood the bug when originally figuring this out. My first solution worked but was really bad.

The Bad / Original Solution

for (var i = 0; i < vehicles.length; i++) {
  $.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
    var vehicle = vehicles.filter(function(theVehicle) { 
      return theVehicle.id === locationData.vehicleId;
    })[0];
    
    vehicle.location = locationData.coords;
  });
}

It works, hooray!

It also forces you to do a read through of the entire array every time that you have a result. This is a relatively expensive operation and performs a lot more work than we need to do; while it may not matter at 5 vehicles, it may matter at 10. Or 50. Or 500. At some point the extra computation will matter, especially when it can be avoided.

The right solution (then)

While I did not know much of JavaScript, one thing I knew was that scopes are based on function. After a great deal of research, I realized that I could just use an immediately invoked function expression in order to create a new scope. Perfect!

The solution that worked looked like this:

for (var i = 0; i < vehicles.length; i++) {
  (function(vehicleIndex) {
    $.get("/vehicles/location" + vehicles[vehicleIndex].id).then(function(locationData) {
      var vehicle = vehicles[vehicleIndex];

      vehicle.location = locationData.coords;
    });
  })(i);
}

In prior loops, when we started the $.get request, the id was proper because vehicles[i] was synchronous, and therefore we were making multiple requests as expected. The same principle applies here!

The only place that vehicleIndex is defined is as a parameter in the immediately invoked function expression, so each iteration of the loop it is re-created with a proper index being passed; when the asynchronous call is completed, vehicleIndex is still correct.

You can watch this occur by using your JS Console:

(function() {
var vehicles = [{id: "some-id-1"}, {id: "some-id-2"}, {id: "some-id-2"}, {id: "some-id-3"}, {id: "some-id-5"}]; 
  
  for (var i = 0; i < vehicles.length; i++) {

    (function(vehicleIndex) {
      setTimeout(function() {
        console.log(vehicleIndex);
        console.log(vehicles[vehicleIndex].id);
      }, 0);
    })(i);

  }
})();

Effectively, the loop was unraveled to be this:

var i = 0;
var vehicles = [{id: "some-id-1"}, {id: "some-id-2"}, {id: "some-id-3"}, {id: "some-id-4"}, {id: "some-id-5"}];

// i = 0;

(function(vehicleIndex) {
  // immediately invoked
  // vehicleIndex is 0;
  $.get("/vehicles/location" + vehicles[vehicleIndex].id).then(function(locationData) {
  // vehicleIndex is never updated; it's still 0
    vehicles[vehicleIndex].location = locationData.coords;
  });
}(i); // passes 0

i = i + 1; 

// i = 1;

(function(vehicleIndex) {
  // immediately invoked
  // vehicleIndex is 1;
  $.get("/vehicles/location" + vehicles[vehicleIndex].id).then(function(locationData) {
  // vehicleIndex is never updated; it's still 1
    vehicles[vehicleIndex].location = locationData.coords;
  });
}(i); // passes 1

i = i + 1; 

// i = 2
// etc

The right solution (now)

The world is a little easier now, and in ES6 the let statement was created; this allows you to create block-specific variables; in the case of the for-loop, you can think of the let keyword as creating a new variable named i at the start of each iteration of the loop that is only accessible to code within each iteration of the loop.

This makes the solution pretty trivial nowadays.

for (let i = 0; i < vehicles.length; i++) {
  $.get("/vehicles/location" + vehicles[i].id).then(function(locationData) {
    vehicles[i].location = locationData.coords;
  });
}

You can witness it in your browser:

(function() {
  var vehicles = [{id: "some-id-1"}, {id: "some-id-2"}, {id: "some-id-2"}, {id: "some-id-3"}, {id: "some-id-5"}]; 
  
  for (let i = 0; i < vehicles.length; i++) {
    setTimeout(function() {
      console.log(i);
      console.log(vehicles[i].id);
    }, 0);
  }
})();

This makes short work of that issue.

Conclusions

Asynchronous code can be tricky!

In our case, the issue was caused as an after-effect of closure; our callback functions had access to the same i variable throughout each of them, which was constantly being updated each iteration of the loop.

We were able to work with this and make anonymous functions or block-level variables in order to avoid this, saving the day without the need for constantly querying our arrays to get the proper elements back.

Currently Drinking: Starbucks Iced Coffee: just because I prefer making my own doesn't mean I can't enjoy good iced coffee when it's made by a chain.