Javascript for loop scope takes last variable

Question

This works:

  var toggler = function(most){
    var open = $('#toggle_' + most + ' .minus').is(':visible');

    if(open){
      $('#toggle_' + most + ' .minus').hide();
      $('#toggle_' + most + ' .plus').show();
    }else{
      $('#toggle_' + most + ' .plus').hide();
      $('#toggle_' + most + ' .minus').show();
    }

    $('#' + most + ' ol.tlist').toggle(open);
  };

  $('#toggle_mostviewed').click(function(){ toggler('mostviewed'); });
  $('#toggle_mostshared').click(function(){ toggler('mostshared'); });
  $('#toggle_mostrecent').click(function(){ toggler('mostrecent'); });

But this does not:

  var toggler = function(most){
    var open = $('#toggle_' + most + ' .minus').is(':visible');

    if(open){
      $('#toggle_' + most + ' .minus').hide();
      $('#toggle_' + most + ' .plus').show();
    }else{
      $('#toggle_' + most + ' .plus').hide();
      $('#toggle_' + most + ' .minus').show();
    }

    $('#' + most + ' ol.tlist').toggle(open);
  };

  var t = ['mostviewed','mostshared','mostrecent'];
  for(var i = 0 ; i  < t.length; i++ ){
    var j = t[i];
    $('#toggle_' + j).click(function(){ toggler(j) });
  }

Is like the for loop was "replaced" by:

  $('#toggle_mostrecent').click(function(){ toggler('mostrecent'); });

i.e. the last iteration is the only that counts.

Closure issue. Google closure for loop. Need to wrap the loop logic in anonymous function. — Phix
– Phix, Commented Nov 9, 2013 at 2:19
Also a variable hoisting misunderstanding. Loop blocks don't get a scope in javascript -- unlike some other languages. That var j isn't where you think it is — J. Holmes
– J. Holmes, Commented Nov 9, 2013 at 2:23
possible duplicate of Javascript closure inside loops - simple practical example — Gabe Moothart
– Gabe Moothart, Commented Nov 9, 2013 at 6:22

Zhube · Accepted Answer · 2013-11-09 02:27:29Z

2

Your loop is constructed incorrectly. When you want to set a variable in a loop to access an element of an array or object, this is the correct syntax:

var test = [];
for(var i = 0; i < test.length; test++)
    (function(index){
        // do cool stuff with test[index]
    })(i);

This creates a closure over the variable i. If you aren't familiar with the syntax, here's what happens:

1) We define a closure (the opening ()'s after the for statement)

2) We define an anonymous function to take the index parameter

3) We pass the index into the closure (i.e. we execute the function)with the final set of ()'s.

These three steps happen for every iteration of the loop. If you don't use the closure to capture the index value, then when the array access is actually made, the index in this example would be +1 too many, and cause errors at runtime.

Cheers

answered Nov 9, 2013 at 2:27

Zhube

5042 silver badges6 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

sites Over a year ago

Thanks a lot, this is the answer I was looking for, are there another ways to handle with this case?

Arun P Johny · Accepted Answer · 2013-11-09 02:19:41Z

0

it is because of wrong use of a closure variable in a loop

In this case since you are iterating through an array, you can use $.each()

var t = ['mostviewed','mostshared','mostrecent'];
$.each(t, function(_,most){
    $('#toggle_' + most).click(function(){ toggler(most) });
})

answered Nov 9, 2013 at 2:19

Arun P Johny

389k68 gold badges532 silver badges532 bronze badges

Comments

jdigital · Accepted Answer · 2013-11-09 02:21:15Z

0

You said:

$('#toggle_' + most).click(function(){ toggler(most) });

but I think this is what you meant:

$('#toggle_' + j).click(function(){ toggler(j) });

(you defined j but then used most instead).

answered Nov 9, 2013 at 2:21

jdigital

12.4k4 gold badges38 silver badges56 bronze badges

Comments

Lucky Soni · Accepted Answer · 2013-11-09 02:23:22Z

0

Why not do something like:

$('#toggle_mostviewed, #toggle_mostshared, #toggle_mostrecent').click(function({
    toggler((this.id).split("_").pop()); 
});

Or even better, give them a class "toggle" (and also leave the IDs in the html) and then:

$('.toggle').click(function({
    toggler((this.id).split("_").pop()); 
});

answered Nov 9, 2013 at 2:23

Lucky Soni

6,8883 gold badges40 silver badges59 bronze badges

Collectives™ on Stack Overflow

Javascript for loop scope takes last variable

4 Answers 4

1 Comment

Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related