CODEWITHSUNDEEP
javascriptjquery
code-8
code-8

Reputation: 58672

Uncaught ReferenceError: i is not defined

I'm trying to make a for-loop base on my array

var lists = [ "a", "b", "c", "d" ];

JS

for ( i = 0; i < lists.length; i++) {

    // console.log(lists[i]);

    $(".sa-report-btn-"+lists[i] ).click(function () {
        $(".sa-hide-"+lists[i]).removeClass("hidden");
        $(".sa-report-"+lists[i]).addClass("hidden");
    });

    $(".sa-hide-btn-"+lists[i]).click(function () {
        $(".sa-hide-"+lists[i]).addClass("hidden");
        $(".sa-report-"+lists[i]).removeClass("hidden");
    });
}

Am I doing it correctly ? I got Uncaught ReferenceError: i is not defined Can I concat each loop with my jQuery selector like this --> $(".sa-hide-"+lists[i]) ? just curious ...

Upvotes: 26

Views: 43630

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074585

First off, it sounds like you're using strict mode — good! It's saved you from falling prey to The Horror of Implicit Globals.

There are two issues with the code.

The first one is that you're missing the declaration for i. You need to add var i; above the loop, e.g:

var i;
for ( i = 0; i < lists.length; i++) {
// ...

or

for (var i = 0; i < lists.length; i++) {

Note, though, that even in that latter example, the i variable is function-wide, not limited to the for loop.

The second one is more subtle, and is outlined in this question and its answers: Your click handlers will have an enduring reference to the i variable, not a copy of it as of where they were created. So when they run in response to a click, they'll see i as the value lists.length (the value it has when the loop has finished).

In your case, it's really easy to fix (and you don't have to declare i anymore): Remove the loop entirely, and replace it with Array#forEach or jQuery.each:

lists.forEach(function(list) {

    $(".sa-report-btn-" + list).click(function () {
        $(".sa-hide-" + list).removeClass("hidden");
        $(".sa-report-" + list).addClass("hidden");
    });

    $(".sa-hide-btn-" + list).click(function () {
        $(".sa-hide-" + list).addClass("hidden");
        $(".sa-report-" + list).removeClass("hidden");
    });
});

If you need to support really old browsers, you can either shim Array#forEach (which was added in 2009, as part of ECMAScript5), or you can use $.each (jQuery.each) instead:

$.each(lists, function(index, list) {
// Note addition ------^
    $(".sa-report-btn-" + list).click(function () {
        $(".sa-hide-" + list).removeClass("hidden");
        $(".sa-report-" + list).addClass("hidden");
    });

    $(".sa-hide-btn-" + list).click(function () {
        $(".sa-hide-" + list).addClass("hidden");
        $(".sa-report-" + list).removeClass("hidden");
    });
});

Note that we don't actually use index anywhere in our callback, but we have to specify it because $.each calls our callback with the index as the first argument, and the value as the second. (Which is why I prefer Array#forEach.) So we have to accept two arguments, with the one we want being the second one.

Upvotes: 42

Related Questions