JavaScript for..in and enum strange behaviour

Question

I have trouble with strange behavior of for..in cycle in JavaScript. I have the following HTML:

<div id="quarter_circle_top_left">...</div>
<div id="quarter_circle_top_right">...</div>
<div id="quarter_circle_bottom_right">...</div>
<div id="quarter_circle_bottom_left">...</div>    

<div id="author_dimension"></div>
<div id="similar_dimension"></div>
<div id="citation_dimension"></div>
<div id="concept_dimension"></div>

What I want is that when I hover over one of the "dimensions" (the latter 4 divs) one of the quarter circles (the first 4 divs) will be highlighted. I have the following enum in JS:

var DIMENSIONS = {
    DIM_AUTHORS: {value: 1, dimCode: "author_dimension", areaCode: "quarter_circle_top_left"},
    DIM_SIMILAR: {value: 2, dimCode: "similar_dimension", areaCode: "quarter_circle_top_right"},
    DIM_CITATIONS: {value: 3, dimCode: "citation_dimension", areaCode: "quarter_circle_bottom_right"},
    DIM_CONCEPTS: {value: 4, dimCode: "concept_dimension", areaCode: "quarter_circle_bottom_left"}
};

And this for..in cycle which should do the highlighting:

for (var key in DIMENSIONS) {
    $("#" + DIMENSIONS[key]['dimCode']).hover(
        function() {
            $("#" + DIMENSIONS[key]['areaCode']).addClass("hover");
        }, 
        function() {
            $("#" + DIMENSIONS[key]['areaCode']).removeClass("hover");
        }
    );
}

This sort of works but what happens is that the "dimensions" don't highlight the respective quarter circle but instead hovering over any of them highlights only the last (bottom left) quarter circle. So when I hover over authors "dimension" I expect the top left quarter circle to be highlighted but instead only the last one (bottom left) is highlighted.

I tried to print the values of DIMENSIONS[key]['dimCode'] and DIMENSIONS[key]['areaCode'] and they are correct. Any ideas why my code doesn't work as expected?

http://stackoverflow.com/questions/1451009/javascript-infamous-loop-problem — Andreas, Jun 22 '12 at 13:56
Proof: http://jsfiddle.net/iambriansreed/YfhYk/ - [:)](http://stackoverflow.com/a/11157757/144665) — iambriansreed, Jun 22 '12 at 14:05

score 3 · Accepted Answer · answered Jun 22 '12 at 13:55

3

As @Cranio says, it's the loop/closure problem.

Since you're using jQuery, you can use $.each:

$.each(DIMENSIONS, function(key, value) {
    // use value['dimCode'] instead of DIMENSIONS[key]['dimcode']
    ...
});

The introduction of the additional function scope avoids the loop/closure problem.

answered Jun 22 '12 at 13:55

Alnitak

334,560
70
407
495

That would make sense as solution, I'm upvoting :) – Cranio Jun 22 '12 at 13:57
@Cranio yeah, in my experience it's a lot more intuitive than having to do the usual "invoke a function that returns a function" method. – Alnitak Jun 22 '12 at 13:57
Even without jQuery or `$.each()` the "invoke a function that returns a function" method is not the easiest way to solve this problem: an immediately invoked anonymous function does the trick in a way that is more efficient: `for (var key in DIMENSIONS) { (function(key) { ... })(key); }` (and in my opinion easier to read). But if using jQuery `$.each()` makes it even easier. – nnnnnn Jun 22 '12 at 14:05
@nnnnnn: How is that more efficient? – Jun 22 '12 at 14:08
@amnotiam: Just as a general note. When benchmarking `each()` vs native for loops, jQuery seems to have a longer execution time. In this particular case off course it makes no difference at all but when using large arrays with complex objects it might start to matter. This article had some benchmarking done in this area: http://jquery-howto.blogspot.ie/2009/06/javascript-for-loop-vs-jquery-each.html. I don't know though if jQuery 1.7 has improved on this or not. I'm only pointing this out as, even though it is tempting, sometimes jQuery may not be the best solution. – Nope Jun 22 '12 at 14:17
@amnotiam: Sorry, I didn't really think it through - this is what happens when I post after midnight. I was thinking a single immediately invoked anonymous function in the loop, inside which would be the `.hover()` (in turn using two anonymous functions) would be more efficient than a `.hover()` that calls two functions that return functions (as in Sirko's answer). Is it too late to argue that it's "more efficient" when coding because it's less typing? I _do_ still think it's _easier_... – nnnnnn Jun 22 '12 at 14:23
@FrançoisWahl: I completely agree. I was asking just how an inline IIFE could be more efficient than invoking a named function that returns a function. The inlined function would be at least theoretically less efficient since the function object needs to be recreated for each iteration. – Jun 22 '12 at 14:24
@nnnnnn: Been there. `:)` Sure, I'll buy the "less typing" interpretation. I personally think the named functions are clearer, but I know that others disagree. – Jun 22 '12 at 14:26
Thanks @amnotiam. Now that you've forced me to think it all through properly I think the point I really wanted to make is simply that using a function that returns a function is needlessly complicated for this problem. A single named function called within the loop would solve the same problem without needing to return anything. (And apologies to Alnitak for cluttering this answer up with a discussion about something only mentioned in a comment and not the answer itself.) – nnnnnn Jun 22 '12 at 14:35
@nnnnnn a named function isn't enough - it also has to be _invoked_ within the loop (and passed the current loop value) to complete the argument binding. – Alnitak Jun 22 '12 at 14:42
Sure - that's what I said ("called within the loop"). – nnnnnn Jun 22 '12 at 21:54

score 2 · Answer 2 · answered Jun 22 '12 at 13:53

2

Because key as a loop variable cannot be referenced in your closure.

answered Jun 22 '12 at 13:53

Cranio

9,647
4
35
55

That is to say that it _can_ be referenced, but it will hold the value as at the end of the loop by the time the hover handlers are called. – nnnnnn Jun 22 '12 at 13:56
Yeah, what I meant was "cannot be referenced the way the OP intended it to work". My bad, good point :) – Cranio Jun 22 '12 at 13:57
1

Thanks a lot for the clarification, I had to read what is a closure but now it makes perfect sense! – robodasha Jun 22 '12 at 14:05

score 2 · Answer 3 · answered Jun 22 '12 at 13:56

Not exactly a fix for your current situation, but I'd suggest you use your key and dimCode as key-value pairs, instead of creating a 2D array.

This should work just fine:

var DIMENSIONS = {
    'author_dimension' : $('#quarter_circle_top_left'),
    // ... so on
};

so you can access that via:

$('div[id*="_dimension"]').hover(function () {
    DIMENSIONS[this.id].addClass('hover');  
}, function () {
    DIMENSIONS[this.id].removeClass('hover');
});

Seems much less complex, and you don't have to do a jQuery selection every time you perform a hover.

Using the ID as the lookup key is by far the best solution. – Jun 22 '12 at 14:18 — , Jun 22 '12 at 14:18

score 1 · Answer 4 · answered Jun 22 '12 at 14:03

1

http://jsfiddle.net/iambriansreed/YfhYk/

for (key in DIMENSIONS) {
    if(!DIMENSIONS.hasOwnProperty(key)) continue;  

    $("#" + DIMENSIONS[key].dimCode)
    .data('areaCode', DIMENSIONS[key].areaCode)
    .hover(function() {
        $("#" + $(this).data('areaCode')).addClass("hover");
    },function() {       
        $("#" + $(this).data('areaCode')).removeClass("hover");
    });
}

answered Jun 22 '12 at 14:03

iambriansreed

21,935
6
63
79

1

@Alnitak I know better than that I promise. Thanks! – iambriansreed Jun 22 '12 at 14:05

score 0 · Answer 5 · answered Jun 22 '12 at 13:55

0

Because key is defined when the loop runs. When the actual Hover event occurs, it isn't set to the same value.

answered Jun 22 '12 at 13:55

phenomnomnominal

5,427
1
30
48

JavaScript for..in and enum strange behaviour

5 Answers5