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)

function 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 ++) { jobQueue . push ( function () { var employee = employees [ i ];

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 ]; ( function ( employee ) { 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 ); } }

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.

function queueEmployeeRaises ( employees , jobQueue , database , multiplier ) { for ( var i = 0 ; i < employees . length ; i ++) { const 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 ); }); } }

Further Reading