JavaScript scoping and choosing names for foreach indices

Question

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.

I think when you say for (var barIndex = 0; i < bar.length; i++) { you mean for (var barIndex = 0; barIndex < bar.length; barIndex++) {. Am I right? I'll delete this comment either way... — Alejandro B., Mar 27 '12 at 22:09
when you do this, you have to make sure you do: `if(potentialDiagramImages.hasOwnProperty(potentialDiagramImagesIndex)){var foo = potentialDiagramImages[potentialDiagramImagesIndex];}` — Eric Hodonsky, Mar 27 '12 at 22:10

score 6 · Accepted Answer · answered Mar 27 '12 at 22:07

6

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]);
}

answered Mar 27 '12 at 22:07

Rob W

341,306
83
791
678

score 4 · Answer 2 · answered Mar 27 '12 at 22:12

4

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.

answered Mar 27 '12 at 22:12

jAndy

231,737
57
305
359

Thanks for the ECMAscript5 update. kinda neat but what's the overhead involved in this vs his approach? – Eric Hodonsky Mar 27 '12 at 22:14
@Relic: quick jsperf http://jsperf.com/foreach-vs-for-sotest. Since there is a function invoked per iteration, the methodical loop is of course slower. But it still performs really fast unless you're doing some graphic-updates or performance critical tasks. – jAndy Mar 27 '12 at 22:17
This is super useful, just FYI! I'm using it all over the place now. Thanks so much! – Sean Anderson Mar 28 '12 at 21:10
@SeanAnderson: yaayy its shiny I know :-) I'm also using it quite everywhere accept, as I mentioned, where you think performance is ultimate important. Have fun! :) – jAndy Mar 28 '12 at 21:21

score 2 · Answer 3 · edited May 23 '17 at 12:06

2

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]);
}

edited May 23 '17 at 12:06

Community

1
1

answered Mar 27 '12 at 22:09

amit_g

30,880
8
61
118

FYI, you left the second `var` in – nrabinowitz Mar 27 '12 at 22:09
@Relic, not everyone types at the same speed and while it is your prerogative to vote up or down, this is not generally the reason to downvote. – amit_g Mar 27 '12 at 22:17
+1 to cancel out @Relic. Posting a duplicate answer within a minute of the first isn't bad, and isn't worth a downvote. See http://meta.stackexchange.com/questions/1096/dealing-with-duplicate-answers for discussion. – nrabinowitz Mar 27 '12 at 22:17
It(SO.com) interrupts you when an answer has been added, don't you read it before you post, and after you post if you are the duplicate wouldn't it make sense to delete your post? Just sayin' – Eric Hodonsky Mar 27 '12 at 22:21
@Relic, I would have deleted it but I believe that the answer had more value in it as it had the reason to why it should be done in addition to what should be done. It is up to the community to decide if it is useful or not (as you have decided that it is not and I respect that even though I don't agree with it). – amit_g Mar 27 '12 at 22:26
@amit_g in that case about accuracy vs info, you're very correct. HOWEVER: that is why there are comments and a community option to edit an answer, so we can give one conclusive answer instead of a bunch of fragment answers. Butt that's just how the site appears to work to me, I could be wrong... but it looks a mess when it's 30 answers. – Eric Hodonsky Mar 27 '12 at 22:32
2

This is really more of a discussion for Meta. But see http://stackoverflow.com/privileges/vote-down - downvotes are to mark bad, sloppy, or incorrect answers, and this isn't one of them. – nrabinowitz Mar 27 '12 at 22:33
@nrabinowitz +1 for true purpose of downvotes; but on the flipside, shouldn't upvotes be based solely on merit and not simply to counteract a downvote you believe is unjustified? – Wiseguy Mar 27 '12 at 22:38
1

@Wiseguy - damn you and your logic :). Probably true. – nrabinowitz Mar 28 '12 at 05:12

score 2 · Answer 4 · answered Mar 27 '12 at 22:10

2

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.

answered Mar 27 '12 at 22:10

Ruan Mendes

90,375
31
153
217

I have to disagree here (though not so strongly that I'd downvote). Declaring the variable twice might lead you to believe that you aren't affecting the value of the previous declaration when you reassign the variable - especially if the variable is used in a closure, this can get disastrous quickly. – nrabinowitz Mar 27 '12 at 22:13
@nrabinowitz: JSHint can give you warnigs if your variables are being used "out of scope". I find that manually hoisting variables silences these useful warnings and just serves to act as a reminder of something I already know. – hugomg Mar 27 '12 at 22:18
@missingno - yeah, I go back and forth on manual hoisting, and generally do it only within relatively small scopes. But declaring a variable twice is, IMHO, sloppy coding, and usually (though not in the OP's case) indicates that you've *actually forgotten* about the first declaration - that seems likely to lead to errors. – nrabinowitz Mar 27 '12 at 22:22
1

@nrabinowitz: You can say it's your opinion, but to call it sloppy coding is just not accurate. Declaring it twice is better than reusing the variable for a different purpose, IMHO, makes it easier to copy/paste into a new method. – Ruan Mendes Mar 28 '12 at 14:52

score 1 · Answer 5 · answered Mar 27 '12 at 22:08

1

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

answered Mar 27 '12 at 22:08

Kevin

532
2
7

2

I'd rather do that only in inner loops – Ruan Mendes Mar 27 '12 at 22:11
This should be left for parent child relationships in loops, his approach is acceptable. – Eric Hodonsky Mar 27 '12 at 22:13

zzzzBov · Answer 6 · 2012-03-27T22:28:46.513

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]);
    }
}());

score 0 · Answer 7 · answered Mar 27 '12 at 22:11

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:

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.
Ignore that warning.

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

JavaScript scoping and choosing names for foreach indices

7 Answers7