Using a loop variable inside a function, javascript scoping confusion

Question

I have built a dropdown menu system, everything works when tested independently, the problem I have is in the code below. I use the jQuery ready function to build the menu bar from an external array (menubar[]). Here I am trying to get the mouseover event to call the dropdown() function, but using a different argument for each anchor tag.

So rolling over the first should call dropdown(0), the second dropdown(1) and so on.

$(document).ready(function () {
    for (i in menubar) {
        var declaration = '<a href="' + baseurl + '/' + menubar[i].url +
                          '" class="menutitle">' + menubar[i].name + '</a>';
        var a = $(declaration).mouseover(function () {
            dropdown(i);
        }).mouseout(function () {
            activeTimer = setTimeout("removedropdowns()", 100);
        });
        $("#menu").append(a);
    }
});

The code is calling dropdown(6); on each rollover. How can I pass the loop variable (i) into the mouseover function as a literal/static value!

I got this working fine in FF by using

.attr('onMouseOver','javascript:dropdown('+i+');')

but that wasn't firing for some versions of IE, so I switched to the jQuery mouseover, which fires, but I have the issue above :(

score 2 · Answer 1 · answered Dec 13 '10 at 21:29

2

Your actual problem is that each of your mouseover callbacks uses the same i you increase i all the way up to 6, the callbacks still point to the same i and therefore all use 6 as the value.

You need to make a copy of the value of i, you can do this by using an anonymous function.

$(document).ready(function () {
    // you should use (for(var i = 0, l = menubar.length; i < l; i++) here in case menubar is an array
    for (var i in menubar) {
        var declaration = '<a href="' + baseurl + '/' + menubar[i].url +
                          '" class="menutitle">' + menubar[i].name + '</a>';


        (function(e) { // e is a new local variable for each callback
            var a = $(declaration).mouseover(function () {
                dropdown(e);

            }).mouseout(function () {
                activeTimer = setTimeout(removedropdowns, 100); // don't use strings for setTimeout, since that calls eval
            });
            $("#menu").append(a);
        })(i); // pass in the value of i
    }
});

answered Dec 13 '10 at 21:29

Ivo Wetzel

46,459
16
98
112

I don't think that an additional anonymous function is needed here. The mouseover handler already is an anonymous function, so we can capture the current value of `i` via that function: `mouseover(function(i) { dropdown(i); })` – Šime Vidas Dec 13 '10 at 21:43
It still uses the same `i` "reference" and therefore gets the current value of `i` which, after the loop is 6. – Ivo Wetzel Dec 13 '10 at 22:06
@Šime Vidas: it's not just using an additional anonymous function, it's that the anonymous function is *executed* (see the line with comment "pass in the value of i") – Nathan Hughes Dec 14 '10 at 13:47
@Nathan @Ivo Yes, I see. My approach does not work (because the event object is assigned to the first parameter of the handler function) – Šime Vidas Dec 14 '10 at 14:20

score 1 · Accepted Answer · answered Dec 13 '10 at 21:17

1

$(function() {
    $(menubar).each(function(i){
        $("#menu").append('<a href="' + baseurl + '/' + menubar[i].url + '" class="menutitle">' + menubar[i].name + '</a>');
    });

    $("#menu a").hover(
        function(){
            dropdown($(this).index());
        },
        function(){
            activeTimer = setTimeout("removedropdowns()", 100);
        }
    );
});

answered Dec 13 '10 at 21:17

hunter

62,308
19
113
113

thankyou, this worked but I had to subtract 1 from the value returned by index() for some reason. – lynks Dec 13 '10 at 21:29
Using `$(selector)` inside a loop is a bad practice. – Šime Vidas Dec 13 '10 at 21:32
Also, using a for loop instead of each() is clearly a better choice: `for (var i=0; i < menubar.length; i++) { $menu.append(...); }` – Šime Vidas Dec 13 '10 at 21:34

James Kovacs · Answer 3 · 2010-12-13T21:40:48.147

0

In JavaScript, if you don't declare your variable, it is defined globally. To fix this, add "var" in front of your i looping variable like this. UPDATE: As Sime noticed (see comment), you also need to pass the variable into the function, otherwise you form a closure on the i.

$(document).ready(function() {
    for(var i in menubar) {
        var declaration = '<a href="' + baseurl + '/' + menubar[i].url + '" class="menutitle">' + menubar[i].name + '</a>';
        var a = $(declaration).mouseover(function(i) { dropdown(i); }).mouseout(function() { activeTimer = setTimeout("removedropdowns()", 100); });
        $("#menu").append(a);
    }
});

edited Dec 13 '10 at 21:40

answered Dec 13 '10 at 21:19

James Kovacs

11,549
40
44

I don't thinks that's enough. As @Alberto said, to capture the current value, you have to pass it as a parameter ... like `function(i) { dropdown(i); }` – Šime Vidas Dec 13 '10 at 21:27
@Sime - Good catch. I hadn't noticed that the variable was being captured by the anonymous function. Updated. – James Kovacs Dec 13 '10 at 21:38
Yea, for every step in the loop, a new mouseover handler function is created which captures the current value of `i` – Šime Vidas Dec 13 '10 at 21:44

score 0 · Answer 4 · answered Dec 13 '10 at 21:20

0

First, don't use for..in but rather ordinary loop.

Second, I would just append the links first then apply the events later:

$(document).ready(function() {
    for (var i = 0; i < menubar.length; i++) {
        $("#menu").append('<a href="' + baseurl + '/' + menubar[i].url + '" class="menutitle">' + menubar[i].name + '</a>');
    }

    $("#menu a").each(function(index) {
        $(this).mouseover(function() { dropdown(index); }).mouseout(function() { activeTimer = setTimeout("removedropdowns()", 100); });
    });
});

answered Dec 13 '10 at 21:20

Shadow The GPT Wizard

66,030
26
140
208

The for loop is for arrays. The for-in loop is for plain objects. The type of the loop depends on the `menubar` variable (if it's an array or not). – Šime Vidas Dec 13 '10 at 21:25
@Šime yep I know.. judging from the code I strongly believe it's ordinary array. – Shadow The GPT Wizard Dec 13 '10 at 21:35
Aha, ok :) But I would note that in the answer. – Šime Vidas Dec 13 '10 at 21:38

score 0 · Answer 5 · edited May 23 '17 at 12:04

0

Have a look here and here.

To capture the current value of i, you need to pass it as a parameter to another function where it can be captured as a local variable:

edited May 23 '17 at 12:04

Community

1
1

answered Dec 13 '10 at 21:21

Alberto Zaccagni

30,779
11
72
106

score 0 · Answer 6 · answered Dec 13 '10 at 21:24

Try using jQuery's each() function:

jQuery(function() {
  jQuery.each(menubar, function(index, element) {
    var declaration = '<a href="' + baseurl + '/' + element.url + '" class="menutitle">' + element.name + '</a>';
    var a = $(declaration).mouseover(function() { dropdown(index); }).mouseout(function() { activeTimer = setTimeout("removedropdowns()", 100); });
    $("#menu").append(a);
  });
});

Using a loop variable inside a function, javascript scoping confusion

6 Answers6