Dealing with duplicate declaration warning for loop variables

Question

Consider the following code:

for (var i=0; i<100; i++) {
    // your code here
}
// some other code here
for (var i=0; i<500; i++) {
    // custom code here
}

Any decent lint tool (jslint, jshint or built in IDE) will tell warning - duplicate declaration of variable i. This can be solved by using variable with another name (k, j) or moving declaration to the top:

var i; // iterator
for (i=0; i<100; i++) {}
for (i=0; i<500; i++) {}

I am not fond of both variants - I don't make declarations at the top usually (and even if I did I wouldn't want see there helper variables - i, j, k) and really nothing bad is going on in those examples to change variables' names for.

Though I do want a huge warning in case I write something like this:

for (var i=0; i<100; i++) {
    for (var i=0; i<500; i++) {} // now that's bad
}

What's your approach to such cases?

Answer 1

JavaScript has many constructions which look like well-known constructions in other computer languages. It's dangerous for JavaScript to interpret the construction in another way as most other computer languages.

If somebody who doesn't know JavaScript good enough (the common case by the way) sees the construction like

for (var i=0; i<100; i++) {
    // your code here
}

or sees declaration of the variable in the block

{
    var i;
    //some code
    {
        var j;
        // some code
    }
}

then most readers will think that block level variables will be defined. JavaScript don't have block level variables. All variables will be interpreted as function level defined.

So I never define variables inside of the code if the code is not just some test code which will be written for 5 min only. The main reason is that I don't want to write code and use language constructions which could be misunderstood.

By the way JSLint finds defining of variables inside of block such bad style that it stop processing of the code analysis. There are no JSLint options which could change this behavior. I find the behavior of JSLint not good, but I agree that declaration of variables inside of for loop is bad because it will be read by most persons as code with local loop variables, which is incorrect.

If you use

for (var i=0; i<100; i++) {
    // your code here
}
// some other code here
for (var i=0; i<500; i++) {
    // custom code here
}

then JavaScript moves all declarations of variables at the beginning of the function for you. So the code will be as

var i = undefined, i = undefined; // duplicate declaration which will be reduced
                                  // to one var i = undefined;

for (i=0; i<100; i++) {
    // your code here
}
// some other code here
for (i=0; i<500; i++) {
    // custom code here
}

So please think about other readers of the code. Don't use any constructions which could be interpreted in the wrong way.

Answer 2

let keyword is introduced in javascript 1.7. Please find MDN documentation here

Replacing var to let inside for loop solves the problem and now one can declare local variable inside the scope of a loop. Check out stackoverlow community explanation here: What's the difference between using "let" and "var" to declare a variable?

Code from future:

for (let i = 0; i < someVar.length; i++) {
    // do your thing here
}

Answer 3

I recommend enclosing your loops within self-executing functions whose names tell you what the loop is doing. This effectively gives block scope to the loop. For example:

var users = response['users']
;(function appendUsers(){
    for (var i = 0; i < users.length; i++) {
        var user      = users[i]
        var userEmail = user['email']
        var userId    = user['id']
        /* etc */ 
    }
})() // appendUsers

If you do this:

var i
for (i = 0; i < someVar.length; i++) {
    for (i = 0; i < someOtherVar.length; i++) {
        // This is a bug that a validator will not notice.
    }
}

On the other hand:

for (var i = 0; i < someVar.length; i++) {
    for (var i = 0; i < someOtherVar.length; i++) {
        // This is a bug that a validator will warn you about.
    }
}
for (var i = 0; i < yetAnotherVar.length; i++) {
    // It will also warn you about this, but so what?
}

You can stop using i as an iterator:

for (var companyIndex = 0; companyIndex < companies.length; companyIndex++) {
    var company = companies[companyIndex]
}

If you're using jQuery anyway, you can use its jQuery.each() method:

$.each(someVar, function(i, value){
    // etc
})

You can't use Array.prototype.forEach() if you want IE8 and earlier to work, unless you add the Polyfill code or similar.

Answer 4

I tend the use the array built-in functions, such as map, filter and forEach, thus avoiding the issue altogether. This also automatically gives you scope loop bodies which is also quite useful.

If using those doesn't match the use case I usually resort to top declaration, simply so I can avoid the issues you mentioned.