JavaScript Function inside the loop

Question

Can someone explain to me why JSLint complains about "Function inside the loop" with this example:

  for (var i = 0; i < buttons.length; i++) {
    (function(i) {
      buttons[i].onclick = function(e) {
        t.progressBars[t.current].update(buttons[i].getAttribute("data-value"));
      }
    })(i);
  }

But dosen't when I change it to:

function makeHandler(i)
  {
    return function() {
        t.progressBars[t.current].update(buttons[i].getAttribute("data-value"));
      }
  }

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

      buttons[i].onclick = makeHandler(i);

  }

I don't quite understand as it seems that with each loop iteration new function object has to be returned, even though it happens inside of makeHandler() function. Why the second example is ok with JS linters?

2nd example binds i with the callback. 1st one doesn't. you will have all i values equal to n in first example. where second one will have 0 to n-1. — Mithun Satheesh, Oct 02 '14 at 03:44
@mithunsatheesh - Look again. The first one is an IFFE and does the same binding. — Ted Hopp, Oct 02 '14 at 03:47
It's just an under-informed warning by jsLint. People give its warnings more credit than it deserves. It can warn you about things you shouldn't be doing, but everything it warns about is not a problem or even necessarily something that should be changed. — jfriend00, Oct 02 '14 at 03:52
@TedHopp: Oh god. am i missing something? [see this fiddle](http://jsfiddle.net/9toxka9o/). doesnt bind. — Mithun Satheesh, Oct 02 '14 at 03:55
@mithunsatheesh - you're alerting the number `1`, not `i`. That doesn't show anything. — jfriend00, Oct 02 '14 at 03:57
@mithunsatheesh - Um, you're calling `alert(1)`; try calling `alert(i)` — Ted Hopp, Oct 02 '14 at 03:57
@mithunsatheesh You have a typo in the fiddle alert(1) should be alert(i) I think — spirytus, Oct 02 '14 at 03:58

score 8 · Answer 1 · edited Jun 20 '20 at 09:12

Quoting from linterrors,

var elems = document.getElementsByClassName("myClass"), i;
for (i = 0; i < elems.length; i++) {
    (function (iCopy) {
        "use strict";
         elems[i].addEventListener("click", function () {
            this.innerHTML = iCopy;
         });
    }(i));
}
What we have now captures the value of i at each iteration of the loop. This happens because JavaScript passes arguments to functions by value. This means that iCopy within the capturing function is not related to i in any way (except for the fact that they happen to have the same value at that point in time). If i changes later (which it does - on the next iteration of the loop) then iCopy is not affected.

This will work as we expect it to but the problem now is that the JavaScript interpreter will create an instance of the capturing function per loop iteration. It has to do this because it doesn't know if the function object will be modified elsewhere. Since functions are standard JavaScript objects, they can have properties like any other object, which could be changed in the loop. Thus by creating the function in the loop context, you cause the interpreter to create multiple function instances, which can cause unexpected behavior and performance problems. To fix the issue, we need to move the function out of the loop:

I would have liked to use Array.prototype.forEach here, like this

buttons.forEach(function(curButton) {
    curButton.onclick = function(e) {
        t.progressBars[t.current].update(curButton.getAttribute("data-value"));
    };
});

Using an IIFE to create a closure is a standard problem solving mechanism in loops. I don't see a problem at all. If you don't need a closure, the code could be moved to an externally defined function so it's a function call instead of an IIFE, but that has some tradeoffs (no access to parent scope variables), extra symbols declared, code not inline for readability, etc... — jfriend00, Oct 02 '14 at 03:54
@jfriend00 Agreed. I was just pointing out what jsLint thinks about code like this :-) — thefourtheye, Oct 02 '14 at 03:56

score 5 · Accepted Answer · answered Oct 02 '14 at 03:41

Your two examples are not equivalent.

In the first, you are creating an anonymous function and calling it on every loop.

The inner function (the click event handler) is fine - you're assigning a new function - but it's the anonymous outer function that is inefficient in this context. In your second example the outer function is refactored out of the loop where is it only created once, instead of buttons.length times.

JavaScript Function inside the loop

2 Answers2

Linked