Why doesn't this javascript bind the correct parameters with the event?

Question

In this code I am supposed to bind a rollover effect to each <area> tag in a <map> element.

function initLinks(webrt) {
  var areas = document.querySelectorAll("map#streetmap > area");
  var links = new Array(areas.length);
  for (var i=0; i<links.length; i++) {
    links[i] = new Image(786,272);
    links[i].src = webrt+"templates/default/sketches/links"+(i+1)+".png";
    areas[i].onmouseover=function(){switchLinkImg(webrt+"templates/default/sketches/links"+(i+1)+".png");};
    areas[i].onmouseout=function(){switchLinkImg(webrt+"templates/default/sketches/links.png");};
  }
}

Strangely, each <area> onmouseover event tries to load the non-existing image: /templates/default/sketches/links6.png. Why does it keep this variable i which has incremented to 6 as a global variable rather than take the string I am passing to the function?

How do I fix this?

Note: No jQuery!

Because there is only *one* variable called `i` and `webrt`. Search for "javascript loop closure". — user2246674, Oct 06 '13 at 20:20
See http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example - either use a "double closure" or `Function.bind`. — user2246674, Oct 06 '13 at 20:20
@user2246674 The confusion I'm having is when I passed the string `webrt+"templates/default/sketches/links"+(i+1)+".png"` shouldn't it have binded that function with the resulting string? — user1094607, Oct 06 '13 at 20:21
I was wrong about the line, but it's the same issue: `areas[i].onmouseover=function(){switchLinkImg(webrt+"templates/default/sketches/links"+(i+1)+".png");};` - the expression (namely, the evaluation of `i`) happens *when* the callback (which is both a closure and an anonymous function) is invoked. The *same* `i` variable is bound for *all* such callbacks/closures created in the code because *only* a new function scope introduces a new variable in JavaScript. — user2246674, Oct 06 '13 at 20:23

score 2 · Answer 1 · answered Oct 06 '13 at 20:40

i often find it cleaner to use array methods when using the index because you don't need extra wrappers and everything reads a little cleaner (imho):

function initLinks(webrt) {
    [].forEach.call(document.querySelectorAll("map#streetmap > area"), 
        function(elm, index){
            var img = new Image(786,272);
            img.src = webrt+"templates/default/sketches/links"+(index+1)+".png";
            elm.onmouseover=function(){switchLinkImg(webrt+"templates/default/sketches/links"+(index+1)+".png");};
            elm.onmouseout=function(){switchLinkImg(webrt+"templates/default/sketches/links.png");};
    });
}

the variable count is way down, and we avoid extra ram-hogging closures by not creating a extra new function in each iteration of the "loop".

to be sure, both ways work, but the newer array methods can also allow the procedure to be recycled by ripping it out of the forEach() call, and giving it a name.

score 1 · Accepted Answer · answered Oct 06 '13 at 20:22

1

Try using the following code:

function initLinks(webrt) {
    var areas = document.querySelectorAll("map#streetmap > area");
    var links = new Array(areas.length);
    for (var i=0; i<links.length; i++) {
        (function(index) {
            links[index] = new Image(786,272);
            links[index].src = webrt+"templates/default/sketches/links"+(index+1)+".png";
            areas[index].onmouseover=function(){switchLinkImg(webrt+"templates/default/sketches/links"+(index+1)+".png");};
            areas[index].onmouseout=function(){switchLinkImg(webrt+"templates/default/sketches/links.png");};
        })(i);
    }
}

You should wrap the i variable into a closure. Otherwise it gets incremented.

answered Oct 06 '13 at 20:22

Krasimir

13,306
3
40
55

On the explanation: `i` *already was bound in a closure*, which is what the problem was. The solution isn't creating a new closure - it's *introducing a new variable scope* (which is then distinct per loop). – user2246674 Oct 06 '13 at 20:26
Thanks, I still find this behavior very strange but at least I know how to avoid it now. – user1094607 Oct 06 '13 at 20:27
1

Yep, my explanation wasn't correct. I meant that you should put the *i* in a new scope. Otherwise it is in the scope of the for loop and the *i*'s value is actually changing there. – Krasimir Oct 06 '13 at 20:28

Why doesn't this javascript bind the correct parameters with the event?

2 Answers2