String doesn’t seem to be passed by value

Question

I use the following function to bind all members of a certain class to a function that changes the current page. Heres the function:

function _bind_menu_items() {
    var menuItems = fja.getElementsByClass("menuItem"),
        index,
        elementID,
        divIdToBind;

    for (index in menuItems) {
        elementID = menuItems[index].id;
        divIdToBind = elementID.replace("Item", "");

        menuItems[index].onclick = function() {
            fja.menu.changeActivePage(divIdToBind);
        };
    }
}

I’ve checked that everything works as expected down to the actual assignment of the onclick property. The error I’m getting is that every div belonging to the menuItem class seems to call the same onclick function... as if the divIdToBind string is the exact same for every onclick that is assigned... How can I fix this?

All of your click event handlers reference the same `divIdToBind` variable (you are creating a function in a loop). This is a quite common problem. Search for *javascript loops closures* and have a look at [Closures for Dummies](http://blog.morrisjohns..com/javascript_closures_for_dummies.html) (Example 5) — Felix Kling, Sep 05 '11 at 08:09
@Thilo: Thanks for finding this. I was always searching for a good duplicate of this situation. This one seems to be good enough. — Felix Kling, Sep 05 '11 at 08:10

score 3 · Accepted Answer · answered Sep 05 '11 at 08:11

Number one mistake for Javascript beginners. you're missing that the anonymous functions which gets bound to your onclick handler, closes over its parent context. Since all closures from the same parent context share the same scope chain, all of this functions will reference the last value which gets passed into divIdToBind.

To solve that issue, the most common workaround is to create another function(-context):

menuItems[index].onclick = (function(id) {
    return function() {
        fja.menu.changeActivePage(id);
    };
}(divIdToBind));

Now we do create another function which gets executed immediately. We pass it in the value from divIdToBind and all it does is to return another function (just to create a new context)

String doesn’t seem to be passed by value

1 Answers1