Loops and Closures
Q: When is a refactoring not a refactoring?
A: When it involves closures in a loop.
JavaScript closures can be quite powerful, and provide useful workarounds for many of the stranger bits of web browser implementations. They’re also far too easy to create unintentionally, and, in my opinion, best avoided unless you’re an expert, or there is no other way.
Anyway, here’s an interesting feature of closures that I didn’t know about before - the variables caught inside the closure remain ‘live’. That is, external changes to the variable are reflected inside the closure. Which can lead to unexpected results, as we’ll see.
Let’s look at this simple piece of code, in which we construct a few DOM nodes, and attach event handlers to them.
<html>
<head>
<script type='text/javascript'>
window.onload=function(){
for (var i=0; i < 3; i++) {
var el = document.createElement("div");
el.style.border='solid red 1px;';
el.style.width='32px';
el.style.height='32px';
el.onclick=function() { alert("i="+i); };
document.body.appendChild(el);
}
}
</script>
</head>
<body>
</body>
</html>
The event handlers use closures to capture an external parameter, in this case the loop variable i. We might expect that the first element would fire an alert saying “i=0″, the second “i=1″ and so on. In fact, all three report “i=3″, that being the value of i after the loop has executed.
At first I thought this might be due to scoping - i isd declared outside of the loop. So I modified it like this:
window.onload=function(){
for (var i=0; i < 3; i++) {
var el = document.createElement("div");
el.style.border='solid red 1px;';
el.style.width='32px';
el.style.height='32px';
var index=i;
el.onclick=function() { alert("i="+i+", index="+i); };
document.body.appendChild(el);
}
but no joy. It reports “i=3, index=2″. Even though ‘index’ is declared inside the loop, the same reference is being passed to all three closures.
Of course, in this case, closures aren’t necessary. Attaching a reference to the DOM element, and providing a single shared event handler does the trick.
window.onload=function(){
for (var i=0; i < 3; i++) {
var el = document.createElement("div");
el.style.border='solid red 1px;';
el.style.width='32px';
el.style.height='32px';
el.index=i;
el.onclick = clicked;
document.body.appendChild(el);
}
}
function clicked(){ alert(this.index); }
But there’s more! If you come across a situation where you realy really need to use closures insdide a loop, just factor out the loop body into a separate function. This forces creation of a new scope, and suddenly the closure works as expected, creating a different index value for each element.
window.onload=function(){
for (var i=0; i < 3; i++) {
createEl(i);
}
}
function createEl(i){
var el = document.createElement("div");
el.style.border='solid red 1px;';
el.style.width='32px';
el.style.height='32px';
el.onclick = function () {
alert(i);
};
document.body.appendChild(el);
}
Another reason to avoid closures unless you really know what you’re doing - pulling ot a method body from a loop ought to be simply a refactoring i.e. improve readability/maintainability/any other-ility, but here it actually alters the behaviour of the code. In a small, clean example like this one it might be obvious enough, but who among us would spot that as the cause of a bug when buriedin the middle of a few hundred lines of code?