Creating a For loop to create multiple 'Click' events in JavaScript/JQuery

Question

I want to create a For loop for a series of 'click' events on my page. I'm creating a timetable where clicking on a Day button will display the events assigned to that day in a div box.
HTML

<div class="cwt-buttons">
<a id="cwt-button1">Monday</a>
<a id="cwt-button2">Tuesday</a>
<a id="cwt-button3">Wednesday</a>
<a id="cwt-button4">Thursday</a>
<a id="cwt-button5">Friday</a>
<a id="cwt-button6">Saturday</a>
<a id="cwt-button7">Sunday</a>
</div>

<div id="cwt-timetable">
<div class="current">Housework</div>
<div class="cwt-Day1">Kickboxing</div>
<div class="cwt-Day2">Homework</div>
<div class="cwt-Day3">Yoga</div>
<div class="cwt-Day4">Eating</div>
<div class="cwt-Day5">Fasting</div>
<div class="cwt-Day6">Running</div>
<div class="cwt-Day7">Funeral</div>
</div>

JS

$(function() {
    for ( var i = 1; i < 8; i++ ) {
        var clickedButton = $("#cwt-button"+i);
        $(clickedButton).click(function() {
        var currentDay = $('#cwt-timetable div.current');
        var selectedDay = $('#cwt-timetable div.cwt-Day'+i);
        currentDay.removeClass('current').addClass('previous');
        (selectedDay).css({ opacity: 0.0 }).addClass('current').animate({ opacity: 1.0 }, 1000,
        function() {
            currentDay.removeClass('previous');
        });     
    })
    }
});

The JavaScript works fine when I have the exact value in e.g. "#cwt-button1"
It just doesn't work when I concatenate the 'i' counter in the loop.

Can anyone see where I'm going wrong? Or am I do something JavaScript can't handle?

You shouldn't have to do this. Just use `$('cwt-buttons a').click(...)` and make your event handler generic enough to work for any link. — user229044, Sep 16 '13 at 02:41
You're also using `$(...)` twice; effectively you're writing `$($('#cwt-button1'))`, which isn't a problem, just unnecessary. — user229044, Sep 16 '13 at 02:42
try this: `var clickedButton = "#cwt-button"+i;`, just a guess. — Cԃաԃ, Sep 16 '13 at 02:53
Hey guys, removing the $() in the variable results in an error, only reason I'm doing it that way. — Khagan, Sep 16 '13 at 03:16

score 2 · Accepted Answer · edited Sep 16 '13 at 03:15

2

This is just the same old issue that gets asked multiple times a day. All your functions created in the loop are created in the same variable scope, so they share the same i variable.

To scope a variable you need a function invocation. jQuery's $.each() is a handy way to do this:

$(function () { // -----------v-----scoped to the function
    $.each(Array(7), function(i) {
        var clickedButton = $('#cwt-button' + (++i));

        $(clickedButton).click(function () {
            var currentDay = $('#cwt-timetable div.current');

            // --------using scoped `i`------------------------v
            var selectedDay = $('#cwt-timetable div.cwt-Day' + i);
            currentDay.removeClass('current').addClass('previous');
            (selectedDay).css({
                opacity: 0.0
            }).addClass('current').animate({
                opacity: 1.0
            }, 1000, function () {
                currentDay.removeClass('previous');
            });
        });
    });
});

edited Sep 16 '13 at 03:15

Khagan

117
5

answered Sep 16 '13 at 02:42

user2736012

3,543
16
13

that is an ugly hack. each was not meant to do this. use an anon function instead – mkoryak Sep 16 '13 at 02:45
@mkoryak: That's silly. First, I am using an anonymous function. Second, this is precisely the benefit of using a functional style iterator. It's not a hack. It's being used exactly as intended. – user2736012 Sep 16 '13 at 02:46
you arent iterating over the array you created, you are using it as a hacky way to get the each to work. i should have said, create an anon function inside the for loop and call it a day – mkoryak Sep 16 '13 at 02:47
@mkoryak: What defines the number of iterations isn't relevant. What we need is a scoped variable, which is what the function invocation provides. – user2736012 Sep 16 '13 at 02:48
@mkoryak: If you're referring to an IIFE, that's a less efficient way to do it since a new function object is created for every iteration of the loop. This way, only one function is created. It's also a less cryptic syntax than the IIFE, which confuses many people, especially when inlined in a loop. – user2736012 Sep 16 '13 at 02:49
you can invoke a function inside of a loop without creating an unneeded array, and without calling each, which is MUCH more overhead then saying `(function(i) { ... }(i))` see: http://stackoverflow.com/questions/1883611/should-i-use-jquery-each – mkoryak Sep 16 '13 at 02:50
@mkoryak: That's an IIFE, and again, you're creating a new function object with every iteration. I'm only creating a single Array object. If we were to use a `for` loop, then a named function would be more appropriate, and much less cryptic. – user2736012 Sep 16 '13 at 02:51
Thanks for the response @user2736012. I've edited the answer to show what code worked for me and accepted it. I would vote up your answer but I don't have the ability yet. – Khagan Sep 16 '13 at 03:36
@Khagan: Good edit. I forgot that the `i` in the handler needed to be incremented. – user2736012 Sep 16 '13 at 03:40

Creating a For loop to create multiple 'Click' events in JavaScript/JQuery

1 Answers1