JSlint - make function within for loop vs. evaluating function

Question

I have a a function that looks like this:

function strip(o) {
  for (var i in o) {
        var test = basic.arr.some(function(x) {return x === i} );
              //JSlint -- don't make functions within loop
        if (test)
        delete o[i];
  }
  return o;
}

the code works and JSlint complains.

I understand that defining multiple functions with a loop' s repeating expression will end up in creating multiple functions each retaining the last repeating expression's value (because each loop doesn't have a distinct scope);

but in this case , since I'm evaluating the function right away, I can't think of a scenario where the actual value of i would change;

I've already resolved using a forEach method but would like to understand why this can eventually lead to problems.

fiddle

This very example is just fine... Since you are creating a method, calling it and forgetting about it synchronously in a single line. Although generally it's a rather error-prone way: creating an anonymous function, that uses a variable outside its local scope, in a loop, is likely to do you harm. I believe the rules of jslint just do not consider such exceptions of the general rule. — Michael Antipin, Jan 25 '16 at 14:43
I think you're over thinking this. You don't define functions in a loop because it isn't DRY. Why not [define the function earlier](http://stackoverflow.com/a/3038555/1028230) as JSLint requests? Aka, "How is this question different than [the usual stuff](http://stackoverflow.com/questions/3037598/how-to-get-around-the-jslint-error-dont-make-functions-within-a-loop?rq=1) on [the same topic](http://stackoverflow.com/questions/3927054/jslint-error-dont-make-functions-within-a-loop-leads-to-question-about-javas?rq=1)?" I think you've got a dupe (but I'll wait to hear f/ you b4 voting to close). — ruffin, Jan 25 '16 at 15:19
@ruffin The main difference here is that the inline function is going to be executed _while the loop runs_, not afterwards. The JSLint warning is trying to protect against closures that may cause problems later down the line, what's presented here is fine and arguably the most correct way it could be written. You _could_ rewrite this code without the inline callback, but it would look (imo) a lot worse for no real benefit. I would use a comment to disable the jslint warning here. — James Thorpe, Jan 25 '16 at 15:27
@JamesThorpe I think this is where you remind yourself of Crockford's quote, "JSLint will hurt your feelings." That said, Crockford doesn't ever tell you to do something that's demonstrably *wrong*. And [MDN disagrees with you in its own example of `some`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some#Examples). /shrug I'd suggest using MDN's own setup for defining functions outside of the `some` and take JSLint's advice. — ruffin, Jan 25 '16 at 15:35
@maioman Explain what you mean by "being called right away" and how that changes JSLint's warning. How does that change the error? Why wouldn't you follow the [setup used in MDN that I referenced above](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some#Examples), which is also what JSLint is suggesting? — ruffin, Jan 25 '16 at 15:38
@ruffin IMHO I'm not declaring (or like jslint says making) any function, I'm calling it and using the return value — maioman, Jan 25 '16 at 15:42
I'm caught up -- It really is a dupe, though. You're redefining that anonymous function *each time you iterate this for loop*: `for (var i in o) {`. Make sense? See fix below, where you don't have to define the function ahead of time. — ruffin, Jan 25 '16 at 16:11

ruffin · Answer 1 · 2016-01-25T16:15:23.447

By enclosing that anonymous function declaration in for (var i in o) {, you're redefining it each time you iterate, not just once, regardless of whether or not it's "invoked immediately" -- and I'd point out your oversight of that redefinition as the reason you want to remove that construct from your code.

So you want to either get rid of for or you need to define the function earlier. I'm going to take "more" of JSLint's advice and get rid of the for as well.

This lints on JSLint.com as is:

/*jslint white:true, devel:true */
var obj = {
  a: true,
  b: true,
  c: true
};

var basic ={
    arr: ['a', 'b']
};

function strip(o) {
  "use strict";
  var test;

  Object.keys(o).forEach(function (i) {
    test = basic.arr.some(function (x) { return x === i; });
    if (test) {
      delete o[i];
    }
  });
  return o;
}
strip(obj);
console.log(obj);

JSlint - make function within for loop vs. evaluating function

1 Answers1