3

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

7
  • 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. Jan 25, 2016 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 as JSLint requests? Aka, "How is this question different than the usual stuff on the same topic?" I think you've got a dupe (but I'll wait to hear f/ you b4 voting to close).
    – ruffin
    Jan 25, 2016 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. Jan 25, 2016 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. /shrug I'd suggest using MDN's own setup for defining functions outside of the some and take JSLint's advice.
    – ruffin
    Jan 25, 2016 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, which is also what JSLint is suggesting?
    – ruffin
    Jan 25, 2016 at 15:38

1 Answer 1

1

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);

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.