1

I have following function:

self.aFunction = function(otherAll, all){
    for(var i = 0; i < all.length; i++){
        ...
    }
    for(var i = 0; i < otherAll.length; i++){
        ...
    }
    return someResult;
};

I have two for loops and they both use the variable "i". They have nothing common and used for each loops separately.

However, i take the following error from jshint:

'i' is already defined.

What is wrong with this? Why is that important to use other variables for each loops in the same function? Is it an issue i should care about in this case?

Asqan
  • 4,319
  • 11
  • 61
  • 100
  • There is nothing wrong with that, but usually you wouldn't have two or more `var` declarations for the same variable. JSHint just points that out. – Felix Kling Oct 04 '16 at 15:46
  • TL;DR the `i` variables are the _same one_, they aren't separate. You can just remove the `var` from the second one, if you wish – VLAZ Oct 04 '16 at 15:46
  • Alternatively, if you don't wish to change your code, change the JSHint rule. – VLAZ Oct 04 '16 at 15:48

1 Answers1

4

In pragmatic terms, it's fine to reuse the same variable in your example, but by declaring it twice, you're giving the impression you think it's scoped to the for loop. It isn't, which is why JSHint is giving you that notice: To make sure that it's not actually a bug in the code.

If you want to reuse i, just declare it once:

self.aFunction = function(otherAll, all){
    var i;
    for(i = 0; i < all.length; i++){
        ...
    }
    for(i = 0; i < otherAll.length; i++){
        ...
    }
    return someResult;
};

Alternately, if all and otherAll are arrays, look at forEach or the many other options in my other answer here.

IF you're able to use ES2015 features (e.g., you know all your target environments support them, or you're transpiling), then:

You can use let:

self.aFunction = function(otherAll, all){
    for(let i = 0; i < all.length; i++){
        ...
    }
    for(let i = 0; i < otherAll.length; i++){
        ...
    }
    return someResult;
};

The i declared with let will be scoped to the for loop (in a very interesting way, no less1).

Or if you don't need the index and just want the values in the loop, and all and otherAll are iterables (arrays are iterable), use let with the new for-of:

self.aFunction = function(otherAll, all){
    for(let value of all){
        ...
    }
    for(let value of otherAll){
        ...
    }
    return someResult;
};

1 "...in a very interesting way, no less...": In for (let i = 0; i < all.length; i++) there's actually a different i for each loop iteration, which gets its value from the value of the previous i just before the increment part of the loop, which is useful if you create closures within the loop — they close over the i for that iteration:

// let...
for (let i = 0; i < 5; ++i) {
  setTimeout(function() {
    console.log("i = " + i);
  }, i * 100);
}

// ...vs. var
setTimeout(function() {
  for (var j = 0; j < 5; ++j) {
    setTimeout(function() {
      console.log("j = " + j);
    }, j * 100);
  }
}, 500);
Community
  • 1
  • 1
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • 2
    I believe the JSHint rule controlling this check is called [shadow](http://jshint.com/docs/options/#shadow) - it can be tweaked, if needed. – VLAZ Oct 04 '16 at 15:50
  • @vlaz: Indeed, though for me, repeated `var` *is* something that should be flagged up. – T.J. Crowder Oct 04 '16 at 15:52
  • @T.J.Crowder I thought so, too, but then I realised it really depends on the context. Sometimes, it's useful to have variables that _conceptually_ belong to one block, even if they might be "redeclared" further down. This is solved using ES6 `let/const` but plain `var`s will be flagged. I do agree that _most of the time_ you'd want that but I don't think blind obedience to the linter is what we should be doing. Tools serve us, not the other way around. The rule can be relaxed for the given method only using in-line annotations, not necessarily everywhere. – VLAZ Oct 04 '16 at 15:59