2

Ive got a problem with the following loop:

for (var i = 0; i < dataElements; ++i){
  d=document.createElement('div');
  $(d).addClass('overviewbit')
  .appendTo('.overview')
  .click(function(){
    id = i;
  });
}

Every Div sets id to the highest value of the loop but i should be the exact value i gets when its created. So the first div should set it to 1, the second div should set it to 2 and so on. i hope you understand my problem and could help me to find a solution.

Frederick Behrends
  • 3,075
  • 23
  • 47

5 Answers5

5

This a common problem. When you create the click handler, it's setting id to i -- the variable, not the value i was storing at the time.

The for loop is completing before any of the DIVs are clicked, so i is equal to the final value from the loop for all the click handlers, and as a result all the ids are set to the same value.

With jQuery you can solve this problem by using .data() storage:

for (var i=0; i<dataElements; ++i){
    d=document.createElement('div');
    $(d).addClass('overviewbit')
        .appendTo('.overview')
        .data('val',i)
        .click(function(){
                id = $(this).data('val'); 
            });
}

However, to do things the "proper" way you would use a JavaScript closure:

for (var i=0; i<dataElements; ++i){
    d=document.createElement('div');
    $(d).addClass('overviewbit')
        .appendTo('.overview')
        .data('val',i)
        .click((function(j){
                return function() { id = j; }
            })(i));
}
Community
  • 1
  • 1
Blazemonger
  • 90,923
  • 26
  • 142
  • 180
  • 1
    Id can't start with a number. You'll have to add a prefix to it, then slice on click, if needed. But it's a +1, tough – Ortiga Oct 13 '11 at 17:35
  • good point -- although some browsers seem to allow numerical IDs, the HTML spec says they shouldn't. – Blazemonger Oct 13 '11 at 17:40
  • id is just some variable, not the id of the div. or at least that's how I read the OP. – Andrew Oct 13 '11 at 17:42
1

I don't quite understand what you're asking, does this do what you want

for (var i = 0; i < dataElements; ++i){
  d=document.createElement('div');
  $(d).addClass('overviewbit')
  .appendTo('.overview')
  .data("id", i)
  .click(function(){
    id = $(this).data("id");
  });
}
TimCodes.NET
  • 4,601
  • 1
  • 25
  • 36
1

You can use this instead,

$.each(dataElements, function(i, el){
  $('<div class="overviewbit"></div>').appendTo('.overview')
  .click(function(){
    id = i;
  });
});
Andrew
  • 13,757
  • 13
  • 66
  • 84
1

This is because when your click function occurs, the loop has already completed. The order jQuery sees it in is:

  1. Loop through, creating and appending the divs.

  2. Loop is done- i is set to the highest value.

  3. A click event occurs. It is at this point that jQuery sets id = i. But since the loop completed first, i is the highest value.

To fix this, you need to set i as a local variable inside the for loop (look into javascript closures):

for (var i = 0; i < dataElements; ++i){
    d=document.createElement('div');
    var myId = i;
    $(d).addClass('overviewbit')
    .appendTo('.overview')
    .click(function(){
          id = myId;
    });
}

You may also want to look at jQuery's each method.

jtfairbank
  • 2,311
  • 2
  • 21
  • 33
0

The issue you are having is the click function does not get evaluated until you actually click that element. This event will probably happen after the for loop is finished. This means i will always be dataElements.length since by the time the click is evaluated the i has been set to the highest index and will remain that way for the remainder of its life.

Keith.Abramo
  • 6,952
  • 2
  • 32
  • 46