jQuery Event Handler created in loop

This is a very common issue people encounter.

JavaScript doesn't have block scope, just function scope. So each function you create in the loop is being created in the same variable environment, and as such they're all referencing the same i variable.

To scope a variable in a new variable environment, you need to invoke a function that has a variable (or function parameter) that references the value you want to retain.

In the code below, we reference it with the function parameter j.

   // Invoke generate_handler() during the loop. It will return a function that
   //                                          has access to its vars/params. 
function generate_handler( j ) {
    return function(event) { 
        switchBanners(j, true);
    };
}
for(var i = 1; i <= totalBanners; i++){
   $('#slider-' + i).click( generate_handler( i ) );
}

Here we invoked the generate_handler() function, passed in i, and had generate_handler() return a function that references the local variable (named j in the function, though you could name it i as well).

The variable environment of the returned function will exist as long as the function exists, so it will continue to have reference to any variables that existed in the environment when/where it was created.

UPDATE: Added var before i to be sure it is declared properly.

Instead of doing something this .. emm .. reckless, you should attach a single event listener and catch events us they bubble up. Its called "event delegation".

Some links:

• http://davidwalsh.name/event-delegate
• http://net.tutsplus.com/tutorials/javascript-ajax/quick-tip-javascript-event-delegation-in-4-minutes/
• http://www.sitepoint.com/javascript-event-delegation-is-easier-than-you-think/
• http://lab.distilldesign.com/event-delegation/

Study this. It is a quite important thing to learn about event management in javascript.

[edit: saw this answer get an upvote and recognized it's using old syntax. Here's some updated syntax, using jQuery's "on" event binding method. The same principle applies. You bind to the closest non-destroyed parent, listening for clicks ON the specified selector.]

$(function() {
  $('.someAncestor').on('click', '.slider', function(e) {
    // code to do stuff on clicking the slider. 'e' passed in is the event
  });
});

Note: if your chain of initialization already has an appropriate spot to insert the listener (ie. you already have a document ready or onload function) you don't need to wrap it in this sample's $(function(){}) method. You would just put the $('.someAncestor')... part at that appropriate spot.

Original answer maintained for more thorough explanation and legacy sample code:

I'm with tereško : delegating events is more powerful than doing each click "on demand" as it were. Easiest way to access the whole group of slider elements is to give each a shared class. Let's say, "slider" Then you can delegate a universal event to all ".slider" elements:

$(function() {
    $('body').delegate('.slider', 'click', function() {
        var sliderSplit = this.id.split('-'); // split the string at the hyphen
        switchBanners(parseInt(sliderSplit[1]), true); // after the split, the number is found in index 1
    });
});

Liddle Fiddle: http://jsfiddle.net/2KrEk/

I'm delegating to "body" only because I don't know your HTML structure. Ideally you will delegate to the closest parent of all sliders that you know is not going to be destroyed by other DOM manipulations. Often ome sort of wrapper or container div.

It's because i isn't evaluated until the click function is called, by which time the loop has finished running and i is at it's max (or worse overwritten somewhere else in code).

Try this:

for(i = 1; i <= totalBanners; i++){
    $('#slider-' + i).click(function(event){    
        switchBanners($(this).attr('id').replace('slider-', ''), true);
    });
}   

That way you're getting the number from the id of the element that's actually been clicked.

Use jQuery $.each

$.each(bannersArray, function(index, element) {
    index += 1; // start from 0
    $('#slider-' + index).click(function(event){    
        switchBanners(index, true);
    });
});

You can study JavaScript Clousure, hope it helps