Sean Anderson
Sean Anderson

Reputation: 29301

JavaScript scoping and choosing names for foreach indices

Consider the following JavaScript:

for (var i = 0; i < foo.length; i++) {
    DoStuff(foo[i]);
}

for (var i = 0; i < bar.length; i++) {
    DoStuff(bar[i]);
}

This code seemed fine to me as a developer coming from a C# background. Unfortunately, this code generates a warning with Visual Studio.

Message 1 'i' is already defined

Okay, sure. It is clear what is happening -- the first declaration of i does not confine i's scope to that of the for loop. I could do a couple of things:

for (var i = 0; i < foo.length; i++) {
    DoStuff(foo[i]);
}

for (i = 0; i < bar.length; i++) {
    DoStuff(bar[i]);
}

I find this solution incorrect due to the fact that the second for loop now has its 'correctness' coupled to that of the first loop -- if I remove the first loop the second loop has to change. Alternatively:

for (var fooIndex = 0; i < foo.length; i++) {
    DoStuff(foo[fooIndex]);
}

for (var barIndex = 0; barIndex  < bar.length; barIndex++) {
    DoStuff(bar[barIndex]);
}

This seems better, and is what I am currently settled on, but I am unhappy with the potentially long names. I make the naming standard of my indices dependent on the variable they are iterating over to guarantee unique name declarations. Unfortunately, If I have a list called "potentialDiagramImages" I don't really want to do:

foreach(var potentialDiagramImagesIndex in potentialDiagramImages){
    var foo = potentialDiagramImages[potentialDiagramImagesIndex];
}

This starts to edge on 'too long of a variable name' in my eyes. I don't know if SO agrees with me, though. In addition, the initial problem still exists with this implementation if I have to iterate over the same list twice in the same scope (for whatever reason).

Anyway, I am just curious how others tackle this scoping dilemma.

Upvotes: 2

Views: 244

Answers (7)

zzzzBov
zzzzBov

Reputation: 179086

JavaScript has functional scope, and due to variable hoisting, the var statement will actually be executed at the top of the containing function. it's advisable to place a single var statement at the top of your scope, and use variables as needed:

(function () {
    "use strict";
    var i,
        l;
    for (i = 0, l = foo.length; i < l; i++) {
        DoStuff(foo[i]);
    }
    for (i = 0, l = bar.length; i < l; i++) {
        DoStuff(bar[i]);
    }
}());

Upvotes: 0

jAndy
jAndy

Reputation: 236032

Just FYI and nobody else mentioned it so far, ECMAscript since edition 5 also offers you some nice little helper methods on the Array.prototype. For instance

foo.forEach(function( elem ) {
    DoStuff( elem );
});

bar.forEach(function( elem ) {
    DoStuff( elem );
});

This way you avoid the confusion with function level scope and variable hoisting. ES5 is widely supported across all browsers, you could (and probably should) include a little ES5-shim library for old'ish browsers anyway.

Upvotes: 4

hugomg
hugomg

Reputation: 69944

As has already mentioned in other answers, something you can do that is not completely horrible is to declare the index variable outside both loops. This uncouples the loops from each other (and you can remover the first one without causing errors) but there are two things I don't like about that: its easy to remove the loops entirely and forget about the variable and moving the variable up silences "variable is used out of scope" warnings.

For this reason there are two alternatives I like:

  1. Use a looping function instead of a for loop

    array.forEach(function(elem){
        doSomething(elem);
    });
    

    This has the additional advantage of not being prone to the closures-inside-for-loops bug.

  2. Ignore that warning.

    I personaly disagree with this particular warning and would consider disabling if that were possible.

Upvotes: 0

Ruan Mendes
Ruan Mendes

Reputation: 92284

I particularly don't care about variables being declared twice. I don't like declaring a variable too far from where it's being used, even if the declaration gets hoisted. You can ignore the warning. The main bug that can happen from hoisting is if you have two statements that declare but don't initialize a variable, since newbies may expect the second declaration to set the variable to undefined.

Like others have suggested, if you really want to make Visual Studio happy, you just declare your index variables at the top of your function, like jslint wants you to.

Upvotes: 2

amit_g
amit_g

Reputation: 31250

Javascript doesn't have block level scope so

for (var i = 0; i < foo.length; i++) {
    DoStuff(foo[i]);
}

creates a variable that is scoped to enclosing function or global if no function encloses it. So the best way is to declare the index variable outside

var i;

for (i = 0; i < foo.length; i++) {
    DoStuff(foo[i]);
}

for (i = 0; i < bar.length; i++) {
    DoStuff(bar[i]);
}

Upvotes: 2

Kevin
Kevin

Reputation: 532

I usually just increment my variables, if I used i next is j, k, l...etc.

Upvotes: 1

Rob W
Rob W

Reputation: 349042

Declare i before the loops (at the top of the function's body):

var i;
for (i = 0; i < foo.length; i++) {
    DoStuff(foo[i]);
}

for (i = 0; i < bar.length; i++) {
    DoStuff(bar[i]);
}

Upvotes: 6

Related Questions