How does JSLint's function-within-a-loop error apply when using jQuery's .each()?

Question

I'm using a jQuery .each() call, which uses an anonymous function, from inside a for loop. JSLint comes up with the warning "Don't make functions within a loop".

Here's a code snippet from a larger function - in essence it is doing a check of whether each player of the game is still "alive" (has at least one piece on the board).

for( i=0 ; i<PLAYERLIMIT ; ++i ) {
    if( player[i].status !== 0 ) { //skip already dead players
        var stillAlive = false;
        $board.find("td").each( function() { //this func causes JSLint warning
            if( $(this).data("owner") === player[i].number ) {
                stillAlive = true;
                return false;
            }
        });
        if( !stillAlive ) {
            //... action to take for dead players
        }
    }
}

I can see how to remove this warning - just declare the function seperately and call it. But this is a very small one-shot function and essentially I consider this the body of a nested for loop, (I'm essentially reading the .each() call as something like for $("td") in $board {})

Is this JSLint providing one of it's style warning, or is this more serious?
Basically, is it best for me to fix this?

I'd like to understand the reason for the warning, so any comments RE the why the warning exists would be useful (again I want to know if it's practical or style).

Answer 1

It's just a style thing mainly, possibly some efficiency gained in IE, though newer JS engines will inline the function via their tracing engine anyway. You can get rid of the JSLint "error" tough, like this:

function setupPlayer(player) {
  var stillAlive = false;
  $board.find("td").each( function() {
      if( $(this).data("owner") === player.number ) {
          stillAlive = true;
          return false;
      }
  });
  if( !stillAlive ) {
      //... action to take for dead players
  }
}

for(var i=0 ; i<PLAYERLIMIT ; ++i ) {
    if( player[i].status !== 0 ) { //skip already dead players
        setupPlayer(player[i]);
    }
}

Should you? Personally I would ignore it in this case for the sake of maintainability, your current version is much easier to read/maintain, to me. Your call here, which version suits you better? Either way is likely to have little impact on performance.

---

Bit of a tangent but related:
What would have a bigger impact on performance is caching that <td> selector, DOM traversal is quite expensive. I'd do this just before your for loop:

var cells = $board.find("td");

And use cells inside the loop, no need finding the same elements again :)

Answer 2

So, I believe JSLint is concerned about something like this:

for (var i=0; i<10; i++) {
    $("#my-elem" + i.toString()).click(function() { alert(i)});
}

That will cause all elements with IDs my-elem0 - my-elem9 to alert "10" when clicked -- i is scoped to the containing function, not to the for loop. JSLint tries to protect you from this by telling you not to make functions in a loop. It's not smart enough to know that each will call your function now, and not later.