September 2008 – mookid on code

Well, it happened when I was making a web page on which the user could pick a number of time slots. The time slots would be configured using a calendar and a couple of time entry controls, and the user would add the time slot by clicking a button. The selected time slots would be stored as a JSON serialized array of time slots in an input element.

The selected time slots would be shown in a list on the page, each one with an X on the side which could be used to remove the appropriate time slot. The problem was, that it did not remove the appropriate time slot. But why?

It’s because of the way JavaScript’s closures are implemented!

You see, my code was something like this (using Crockford’s JSON library and the incredible jQuery):

// get array of selected time slots
var timeSlots = JSON.parse($('#selectedTimeSlotsJson').val());

// create representation, including a link to remove each time slot
for(var index = 0; index < timeSlots.length; index++) {
    var timeSlot = timeSlots[index];
    var id = timeSlot.id;

    var removeTimeSlotLink = $('<img>')
        .attr('src', deleteTimeSlotImagePath)
        .bind('click', function() { removeTimeSlot(id); })
        .addClass('hasPointer');

    // more code here to create and attach a DOM element containing
    // a string representation of the time slot and the removeTimeSlotLink
    // (...)
}

// get array of selected time slots

var timeSlots = JSON.parse($('#selectedTimeSlotsJson').val());

// create representation, including a link to remove each time slot

for(var index = 0; index < timeSlots.length; index++) {

var timeSlot = timeSlots[index];

var id = timeSlot.id;

var removeTimeSlotLink = $('<img>')

.attr('src', deleteTimeSlotImagePath)

.bind('click', function() { removeTimeSlot(id); })

.addClass('hasPointer');

// more code here to create and attach a DOM element containing

// a string representation of the time slot and the removeTimeSlotLink

// (...)

}

What happened when I tried to remove the time slots? The most recently added time slot would get removed all the time, even though I clicked the first and the second etc.

At this point, I knew it was due to some JavaScript function closure stuff, because I could see that my click event handler would call removeTimeSlot with the id of the most recently added time slot all the time.

But why was this happening? I made all kind of changes to the snippet, moved the variables around, made several temporary variables, but the problem persisted!

Then I searched around, and I discovered two things that I did not know about JavaScript (and would not have expected):

Variables reside in the function in which they are declared.
Functions get the closure of their containing function.

I am a C# guy, and in C# I know that an anonymous delegate gets the closure that it needs and nothing more. I also know that variables declared inside the scope of for instance a for loop are local to that loop. So doing stuff like this would not cause any unexpected behaviour:

foreach(var timeSlot in timeSlots) {
    // create local reference to be lifted out of this scope
    var timeSlotTemp = timeSlot;

    var button = new Button();
    button.Click += delegate { RemoveTimeSlot(timeSlotTemp); };
    
    // place button somewhere
    // (...)
}

foreach(var timeSlot in timeSlots) {

// create local reference to be lifted out of this scope

var timeSlotTemp = timeSlot;

var button = new Button();

button.Click += delegate { RemoveTimeSlot(timeSlotTemp); };

// place button somewhere

// (...)

}

To put it another way, it means that my code was actually equivalent to this (notice how all my variabled are “declared” at the top of the function):

// get array of selected time slots
var timeSlots = JSON.parse($('#selectedTimeSlotsJson').val());
var index;
var timeSlot;
var id;
var removeTimeSlotLink;

// create representation, including a link to remove each time slot
for(index = 0; index < timeSlots.length; index++) {
    timeSlot = timeSlots[index];
    id = timeSlot.id;

    removeTimeSlotLink = $('<img>')
        .attr('src', deleteTimeSlotImagePath)
        .bind('click', function() { removeTimeSlot(id); })
        .addClass('hasPointer');

    // more code here to create and attach a DOM element containing
    // a string representation of the time slot and the removeTimeSlotLink
    // (...)
}

// get array of selected time slots

var timeSlots = JSON.parse($('#selectedTimeSlotsJson').val());

var index;

var timeSlot;

var id;

var removeTimeSlotLink;

// create representation, including a link to remove each time slot

for(index = 0; index < timeSlots.length; index++) {

timeSlot = timeSlots[index];

id = timeSlot.id;

removeTimeSlotLink = $('<img>')

.attr('src', deleteTimeSlotImagePath)

.bind('click', function() { removeTimeSlot(id); })

.addClass('hasPointer');

// more code here to create and attach a DOM element containing

// a string representation of the time slot and the removeTimeSlotLink

// (...)

}

To circumvent this, I created an inline function which would deliver the DOM element with the link. Something like this:

// get array of selected time slots
var timeSlots = JSON.parse($('#selectedTimeSlotsJson').val());
var index;
var timeSlot;
var removeTimeSlotLink;

var createRemoveTimeSlotLink = function(id) {
    // id is safe in here... :-)
    return $('<img>')
        .attr('src', deleteTimeSlotImagePath)
        .bind('click', function() { removeTimeSlot(id); })
        .addClass('hasPointer');
};

// create representation, including a link to remove each time slot
for(index = 0; index < timeSlots.length; index++) {
    timeSlot = timeSlots[index];

    removeTimeSlotLink = createRemoveTimeSlotLink(timeSlot.id);

    // (...)
}

// get array of selected time slots

var timeSlots = JSON.parse($('#selectedTimeSlotsJson').val());

var index;

var timeSlot;

var removeTimeSlotLink;

var createRemoveTimeSlotLink = function(id) {

// id is safe in here... :-)

return $('<img>')

.attr('src', deleteTimeSlotImagePath)

.bind('click', function() { removeTimeSlot(id); })

.addClass('hasPointer');

};

// create representation, including a link to remove each time slot

for(index = 0; index < timeSlots.length; index++) {

timeSlot = timeSlots[index];

removeTimeSlotLink = createRemoveTimeSlotLink(timeSlot.id);

// (...)

}

– and from now on, to avoid unnecessary confusion, I will stop declaring my variables anywhere except the beginning of each function.

Lastly, I would like to give some shoutouts to the Mozilla folks for driving us all forward. Firefox 2 actually implements JavaScript 1.7, in which let was introduced as a keyword. let creates a new lexical scope and fixes one or more variables, which will then be promoted to a closure if necessary. Using let, my code would look like this:

// get array of selected time slots
var timeSlots = JSON.parse($('#selectedTimeSlotsJson').val());

// create representation, including a link to remove each time slot
for(var index = 0; index < timeSlots.length; index++) {
    var timeSlot = timeSlots[index];
    
    // introduce a new scope where id resides
    let (id = timeSlot.id) {
        var removeTimeSlotLink = $('<img>')
            .attr('src', deleteTimeSlotImagePath)
            .bind('click', function() { removeTimeSlot(id); })
            .addClass('hasPointer');
    }

    // more code here to create and attach a DOM element containing
    // a string representation of the time slot and the removeTimeSlotLink
    // (...)
}

// get array of selected time slots

var timeSlots = JSON.parse($('#selectedTimeSlotsJson').val());

// create representation, including a link to remove each time slot

for(var index = 0; index < timeSlots.length; index++) {

var timeSlot = timeSlots[index];

// introduce a new scope where id resides

let (id = timeSlot.id) {

var removeTimeSlotLink = $('<img>')

.attr('src', deleteTimeSlotImagePath)

.bind('click', function() { removeTimeSlot(id); })

.addClass('hasPointer');

}

// more code here to create and attach a DOM element containing

// a string representation of the time slot and the removeTimeSlotLink

// (...)

}

– which is a lot simpler than the inline function version I ended up using. However, JavaScript 1.7 is not implemented by other browsers than Firefox 2 and Safari 3.x (according to Wikipedia) so it’s not really an option yet.

Month: September 2008

How JavaScript’s closures surprised me