Variables in loop

Question

carousel: function(){
            var $carouselCr = $('#carousel'),
                $tabCr = $('.carouselTabs', $carouselCr),
                $itemCr = $('.carouselContents', $carouselCr),
                tabAmount = (function(){
                    if($('a', $tabCr).length === $('.item', $itemCr).length){
                        return $('a', $tabCr).length;
                    }else{
                        throw "error: verschillend aantal tabs vs items";
                    }               
                })();

            var i = tabAmount;
            while(i--){                                     
                var item = $($('.item', $itemCr)[i]),
                    tab = $($('a', $tabCr)[i]);
                console.log(item, tab);
                $(tab).click(function(){
                    $('.item', $itemCr).hide();
                    $(item).show();
                })

            }

        }

As you can see, i'm trying to attach a click event to each 'tab', to select each 'item'. I'm doing something wrong. All the tabs refer to the first item.

If i log $('.item', $itemCr)[i] inside the loop it will return all the different items, not just the first.

Simplified HTML structure

<div id="carousel" class="block">
    <div class="carouselTabs">
        <a href="#">
        </a>
    <!-- repeating -->
    </div>
    <div class="carouselContents">                      
        <div class="item">
        </div>  
    <!-- repeating -->                  
    </div>
</div>

possible duplicate of [Access outside variable in loop from Javascript closure](http://stackoverflow.com/questions/1331769/) and many, *many*, ***MANY*** others. — outis, Jan 13 '12 at 14:43

score 3 · Accepted Answer · 2012-01-13T15:43:21.553

A loop doesn't create a new variable scope. You need to create the click handler in a separate function, and pass whatever needs to be scoped into that function.

 // creates the handler with the scoped item, and returns the handler
function create_handler(this_item) {
    return function () {
        $('.item', $itemCr).hide();
        $(this_item).show();
    };
}

var i = tabAmount;
var a_els = $('a', $tabCr);
var items = $('.item', $itemCr);
while (i--) {
    var item = items[i],
        tab = a_els[i];
    $(tab).click( create_handler(item) );
}

Also note that you should not do DOM selection in a loop. Cache it once outside the loop, and reference it in the loop as I did above.

It seems that there have been some changes to the code from the original question. I'd rewrite the code like this:

carousel: function(){
    var $carouselCr = $('#carousel'),
        $tabCr = $('.carouselTabs', $carouselCr),
        $itemCr = $('.carouselContents', $carouselCr),
        $items = $('.item', $itemCr),
        $a_els = $('a', $tabCr);

    if($a_els.length !== $items.length)
        throw "error: verschillend aantal tabs vs items";

    $a_els.each(function(i) {
        $(this).click(function() {
            $items.hide();
            $items.eq(i).show();
        });
    });
}

Now each .click() handler is referencing a unique i, which is the index of the current $a_els element in the iteration.

So for example when a click happens on the $a_els at index 3, $items.eq(i).show(); will show the $items element that is also at index 3.

Another approach is to use event delegation, where you place the handler on the container, and provide a selector to determine if the handler should be invoked.

If you're using jQuery 1.7 or later, you'd use .on()...

carousel: function(){
    var $carouselCr = $('#carousel'),
        $tabCr = $('.carouselTabs', $carouselCr),
        $itemCr = $('.carouselContents', $carouselCr),
        $a_els = $('a', $tabCr),
        $items = $('.item', $itemCr);

    if($a_els.length !== $items.length)
        throw "error: verschillend aantal tabs vs items";

    $tabCr.on('click','a',function() {
        var idx = $a_els.index( this ); // get the index of the clicked <a>
        $items.hide();
        $items.eq(idx).show(); // ...and use that index to show the content
    });
}

Or before jQuery 1.7, you'd use .delegate()...

carousel: function(){
    var $carouselCr = $('#carousel'),
        $tabCr = $('.carouselTabs', $carouselCr),
        $itemCr = $('.carouselContents', $carouselCr),
        $a_els = $('a', $tabCr),
        $items = $('.item', $itemCr);

    if($a_els.length !== $items.length)
        throw "error: verschillend aantal tabs vs items";

    $tabCr.delegate('a','click',function() {
        var idx = $a_els.index( this ); // get the index of the clicked <a>...
        $items.hide();
        $items.eq(idx).show(); // ...and use that index to show the content
    });
}

This way there's only one handler bound to the $tabCr container. It check to see if the item clicked matches the 'a' selector, and if so, it invokes the handler.

If there are other elements in between the <a>...</a> elements or the <div class="item">...</div> elements so that the indices don't naturally match up, we'd need to tweak the .index() call just a bit.

Thanks for all the suggestions! Using .each() is actually what i usually do. I thought i read somewhere that using a regular javascript loop is faster. — dubbelj, Jan 13 '12 at 15:06
@dubbelj: Yes, a `for` statement would normally be faster, because `.each()` needs to invoke a function on each iteration of the loop. But since you need to invoke a function anyway to create the variable scope, then it doesn't really matter which you use. There are probably ways to accomplish what you need without scoping variables, but we would need to see your HTML structure. — , Jan 13 '12 at 15:11
I understand. I added some HTML structure. Don't feel obligated to go into this any further. My problems are solved. — dubbelj, Jan 13 '12 at 15:24
@dubbelj: I added an event delegation solution at the bottom. It is a little different depending on if you're using jQuery 1.7, or an earlier version. Enjoy! :) — , Jan 13 '12 at 15:45

score 1 · Answer 2 · answered Jan 13 '12 at 15:42

to simplify your code and to make it more performant you can use a delegate method

$('.carouselTabs', '#carousel').delegate('a', 'click', function(){

  var ind = $('.carouselTabs a').index(this);
  $('.item', '#carousel').hide().eq(ind).show();

  return false;
});

Variables in loop

2 Answers2