Understanding JavaScript Closures in For Loops

How do you pass a variable into a function inside a loop? It always ends up taking the last value! What gives?

Let's suppose you're lucky enough to work for a company that gives annual cost-of-living pay increases. Let's also suppose the corporate payroll system is written in JavaScript. (Why not? everything seems to be JavaScript these days)

And, because we're big enterprise, our system is rife with inefficiency and we've over-engineered everything. Below is what the payroll function looks like. (This is a totally contrived example... so you'll have to practice suspension of disbelief rather than ask why it's designed so poorly)

// Queue jobs to assign employee pay increasesfunction queueEmployeeRaises(employees, jobQueue, database, multiplier) {  for (var i = 0; i < employees.length; i++) {    var employee = employees[i];    jobQueue.push(function() {      if (employee.title == 'CEO') {        employee.salary = employee.salary * Math.max(1.20, multiplier);      }      else {        employee.salary = employee.salary * Math.min(multiplier, 1.02);      }      database.save(employee);    });  }}

What is wrong with this code?

Aside from the horrendous architecture (e.g. violation of SRP)

Aside from the socio-economic disparity between America's working classes...

We are pushing "jobs" onto a queue -- which by itself is fine. Part of the problem is that our "job" function takes variables from outside and uses them inside.

This is called a closure.

It is said that the job function closes over, or has captured, employee -- which just means that we've saved a reference to the variable.

Closures are also fine, when used properly. But the other part of the problem is that we've got a loop that is changing the variable on which the job function has closed over.

Think of our reference to employee as we're holding onto a box. When we go to use the employee value, what we're really doing is opening the box and taking out what's inside... and the for-loop is changing what's inside the box.

Stepping through the problem

Suppose we've got the following employees:

var employees = [  {    name: 'Alice',    title: 'Jr. Software Developer',    salary: 65000,  },  {    name: 'Bob',    title: 'Sr. Software Developer',    salary: 90000,  },  {    name: 'Charlie',    title: 'CEO',    salary: 5000000,  },];

Okay, so we've got two software developers and a CEO.

Now imagine you're single-stepping through the function with our employee array. This is what memory looks like as the code executes:

OMG! I think we've just discovered how CEO salaries have become so disproportionately high!

Here is a rough pseudocode account of what happens as the code executes...

    call queueEmployeeRaises
      allocate memory for i
      allocate memory for employee
      set i to 0
      compare i < employees.length // 0 < 3
      set employee to Alice
      create closure #1
      call jobQueue.push
      set i++
      compare i < employees.length // 1 < 3
      set employee to Bob
      create closure #2
      call jobQueue.push
      set i++
      compare i < employees.length // 2 < 3
      set employee to Charlie
      create closure #3
      call jobQueue.push
      set i++
      compare i < employees.length // 3 == 3
      break out of loop
      memory for i deallocated
      memory for employee persists because it's been captured in closures
    end queueEmployeeRaises
    
    ... // some time later the 'jobs' are executed
    
    call closure #1
      lookup employee in memory // Charlie
      compare employee.title == 'CEO' // true
      set employee.salary = employee.salary * 1.20 // $5MM + 20% = $6MM
      call database.save
    end closure #1
    call closure #2
      lookup employee in memory // Charlie
      compare employee.title == 'CEO' // true
      set employee.salary = employee.salary * 1.20 // $6MM + 20% = $7.2MM
      call database.save
    end closure #2
    call closure #3
      lookup employee in memory // Charlie
      compare employee.title == 'CEO' // true
      set employee.salary = employee.salary * 1.20 // $7.2MM + 20% = $8.64MM
      call database.save
    end closure #3
    

Yeah, well what if...

Maybe now you're thinking that you can just move var employee = employees[i] inside the closure...

function queueEmployeeRaises(employees, jobQueue, database, multiplier) {  for (var i = 0; i < employees.length; i++) {    // var employee = employees[i];    jobQueue.push(function() {      var employee = employees[i]; // How about this?

Nope, still wrong.

We've only moved the problem from employee to i. The closure is now closing over i and the loop is still changing the value.

This is actually worse because i will hold the value 3 when our jobs run and, because arrays are 0-indexed, employees[3] === undefined so we'll get an error "can't read property title from undefined".

Raises for nobody! Sorry, CEO.

ES5 Solution: Change the Closure

Instead of closing over something that changes, we're going to close over something that doesn't change.

function queueEmployeeRaises(employees, jobQueue, database, multiplier) {  for (var i = 0; i < employees.length; i++) {    var e = employees[i]; // changed to e    (function(employee) { // added this. employee is now a parameter      jobQueue.push(function() {        if (employee.title == 'CEO') {          employee.salary = employee.salary * Math.max(1.20, multiplier);        }        else {          employee.salary = employee.salary * Math.min(multiplier, 1.02);        }        database.save(employee);      });     })(e); // added this. Call the function with current employee in e  }}

Wait a minute... Doesn't the parameter still change each time through the loop?

Yes. The key difference here is that the function we added is executing immediately.

i.e. It's running inside the loop as opposed to after the loop has already completed. The closure is now closing over the parameter to our function, which is effectively fixed to the value it was called with rather than the future value of e.

Note: We didn't have to change the variable name from employee to e. I just did that for clarity. We could have left the variable name alone and then there would have been an employee at the queueEmployeeRaises scope and another in the anonymous inner function scope.

ES6 Solution: Buh bye, var

Using the var keyword causes the memory to be allocated once in the scope of queueEmployeeRaises.

We could have solved the problem in ES6 had we used the new let or const keywords, in which case separate memory would have been allocated each time through the loop and there would have been no problem.

// Queue jobs to assign employee pay increasesfunction queueEmployeeRaises(employees, jobQueue, database, multiplier) {  for (var i = 0; i < employees.length; i++) {    const employee = employees[i]; // const instead of var    jobQueue.push(function() {      if (employee.title == 'CEO') {        employee.salary = employee.salary * Math.max(1.20, multiplier);      }      else {        employee.salary = employee.salary * Math.min(multiplier, 1.02);      }      database.save(employee);    });  }}

Further Reading

• Closures
• let keyword
• const keyword