63

I am working on making all of our JS code pass through jslint, sometimes with a lot of tweaking with the options to get legacy code pass for now on with the intention to fix it properly later.

There is one thing that jslint complains about that I do not have a workround for. That is when using constructs like this, we get the error 'Don't make functions within a loop.'

for (prop in newObject) {
    // Check if we're overwriting an existing function
    if (typeof newObject[prop] === "function" && typeof _super[prop] === "function" &&
        fnTest.test(newObject[prop])) {
        prototype[prop] = (function(name, func) {
            return function() {
                var result, old_super;

                old_super = this._super;
                this._super = _super[name];
                result = func.apply(this, arguments);
                this._super = old_super;

                return result;
            };
        })(prop, newObject[prop]);
    }
}

This loop is part of a JS implementation of classical inheritance where classes that extend existing classes retain the super property of the extended class when invoking a member of the extended class. Just to clarify, the implementation above is inspired by this blog post by John Resig.

But we also have other instances of functions created within a loop.

The only workaround so far is to exclude these JS files from jslint, but we would like to use jslint for code validation and syntax checking as part of our continuous integration and build workflow.

Is there a better way to implement functionality like this or is there a way to tweak code like this through jslint?

Rishabh
  • 3,752
  • 4
  • 47
  • 74
Ernelli
  • 3,960
  • 3
  • 28
  • 34

6 Answers6

68

Douglas Crockford has a new idiomatic way of achieving the above - his old technique was to use an inner function to bind the variables, but the new technique uses a function maker. See slide 74 in the slides to his "Function the Ultimate" talk. [This slideshare no longer exists]

For the lazy, here is the code:

function make_handler(div_id) {
    return function () {
        alert(div_id);
    };
}
for (i ...) {
    div_id = divs[i].id;
    divs[i].onclick = make_handler(div_id);
}
havlock
  • 662
  • 9
  • 16
Skilldrick
  • 69,215
  • 34
  • 177
  • 229
  • 20
    How is a "function maker" any better than creating a new function per loop iteration? Isn't it the same thing? – Gili Dec 19 '12 at 16:59
  • 11
    @Gili: The code to create the make_handler function is executed at the point it says `function make_handler(div_id)`. Once inside the `for` loop, `make_handler` is now a reference to that function and is not being re-created for every iteration of the loop. – Ash Clarke Feb 19 '13 at 17:13
  • Wouldn't it be more efficient to allocate the function into a var – Pascalius Mar 11 '13 at 13:17
  • Since slides are still down, watch the video (starting at 1:06:15) - http://www.youtube.com/watch?v=ya4UHuXNygM – Chris Herring Sep 04 '13 at 02:22
  • What is the point of wrapping a function inside a function - why not just declare the function above? Is there some advantage I am missing? -- Ah I see it is only for something like handlers which are not called straight away (so not relevant for OP's example) – Dominic May 13 '14 at 20:51
  • 21
    @AshClarke It is correct that `make_handler` is not recreated for each iteration of the loop but it remains that for each iteration of the loop **the value returned by `make_handler` is a new function object**. What the code in this answer does is just *hide* from a code checker that a new function is created with each iteration but does not *prevent* the creation of a new function with each iteration. Compare the return values of multiple calls to `make_handler` with `===` and see for yourself. – Louis Oct 13 '14 at 12:56
  • 2
    @Louis Yes, true; I should have been more explicit. I was indeed trying to get across that the creation of the `make_handler` function was not evaluated over and over, but that is a good point I should have made about the return value. – Ash Clarke Oct 14 '14 at 14:27
  • 1
    The function maker is better because it will actually work properly. See http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example – Vadim Jan 09 '17 at 23:08
13

(I just stumbled on this questions many months after it was posted...)

If you make a function in a loop, an instance of a function is created for each iteration of the loop. Unless the function that is being made is in fact different for each iteration, then use the method of putting the function generator outside the loop -- doing so isn't just Crockery, it lets others who read your code know that this was your intent.

If the function is actually the same function being assigned to different values in an iteration (or objects produced in an iteration), then instead you need to assign the function to a named variable, and use that singular instance of the function in assignment within the loop:

handler = function (div_id) {
    return function() { alert(div_id); }
}

for (i ...) {
    div_id = divs[i].id;
    divs[i].onclick = handler(div_id);
}

Greater commentary/discussion about this was made by others smarter than me when I posed a similar question here on Stack Overflow: JSlint error 'Don't make functions within a loop.' leads to question about Javascript itself

As for JSLint: Yes, it is dogmatic and idiomatic. That said, it is usually "right" -- I discover that many many people who vocalize negatively about JSLint actually don't understand (the subtleties of) Javascript, which are many and obtuse.

Community
  • 1
  • 1
Zhami
  • 19,033
  • 14
  • 48
  • 47
  • please add vars for the variables (handler, div_id). – Ohad Kravchick Jan 09 '12 at 22:37
  • 1
    when you invoke handler(div_id) you actually run the function which will show the alert. This will happen when the for runs and not when the user clicks on the div. – Ohad Kravchick Jan 09 '12 at 22:38
  • 1
    While there are no vars declared, this is pseudo-code...also when you invoke handler(div_id); it returns a function, it will not run the alert until clicked in this example. Here is a full example: http://jsfiddle.net/scottux/RWCje/ – Scottux May 31 '12 at 01:19
  • +1 but I consider this a false-positive when the function actually uses different variables per loop iteration. Using a "function maker" is no better than instantiating it inside the loop. – Gili Dec 19 '12 at 17:01
11

Literally, get around the problem by doing the following:

  1. Create a .jshintrc file

  2. Add the following line to your .jshintrc file

    {"loopfunc" : true, // tolerate functions being defined in loops }

lifebalance
  • 1,846
  • 3
  • 25
  • 57
  • I see your point. If you take the question literally it's the right answer actually. The down vote was because I think it's bad advise. Anyone asking this question needs to be educated instead. SO doesn't let me revert my down vote unless the answer is edited... – Thijs Koerselman Nov 02 '16 at 08:43
8

JSLint is only a guide, you don't always have to adhere to the rules. The thing is, you're not creating functions in a loop in the sense that it's referring to. You only create your classes once in your application, not over and over again.

Evan Trimboli
  • 29,900
  • 6
  • 45
  • 66
  • 1
    I know that jslint is only a recomendation and that it's intended to be used to spot bad JS code, but it's still a good tool to use to pre-qualify and sanity check your code in an automated build/test environment. As I wrote, I'm interested in workarounds to make the code pass jslint, not avoid running jslint at all. – Ernelli Jun 14 '10 at 14:12
  • 11
    Sure, but if you look at the answer below, you're still making a function in a loop. You're adding extra code just to appease JSLint. Why would you do this? – Evan Trimboli Jun 15 '10 at 04:35
  • 1
    Yes I still make functions within a loop, my problem was that I wanted this code to pass jslint. Now Skilldrick and Matt Eberts gave me working solutions which I have tested and now my code both works and passes jslint. I am an old C/C++ programmer, I'm used to have syntax checking as part of the compilation stage. Javascript lacks compilation, jslint is as close as you get to using a modern c/c++ compiler with warnings. – Ernelli Jun 15 '10 at 15:12
  • 8
    Of course, those solutions are obvious. The point remains, you're still creating functions inside the loop, you've just written it in a particular way that is arguably more convoluted, just to appease some tool. Whatever, if that's what you want to do, go for it. – Evan Trimboli Jun 15 '10 at 16:45
  • Maybe this is covered by someone else in this discussion but I don't see it: I feel like the proposed solution is better with the implication that you're binding a function in a loop, not creating it. Technically, you are (obviously), but it was my understanding that JSLint gives you this hint because functions are supposed to be available at runtime, before the iterator or limit of a loop is defined. In this case, if the iterator results in 0 iterations of the loop, the function is still declared but not bound. As far as how javascript actually interprets this, I'm assuming that part is moot. – NateDSaint May 14 '12 at 19:54
5

If you are using JQuery, you might want to do something like this in a loop:

for (var i = 0; i < 100; i++) {
  $("#button").click(function() {
    alert(i);
  });
}

To satisfy JSLint, one way to work around this is (in JQuery 1.4.3+) to use the additional handler data argument to .click():

function new_function(e) {
  var data = e.data; // from handler
  alert(data); // do whatever
}

for (var i = 0; i < 100; i++) {
  $("#button").click(i, new_function);
}
jevon
  • 3,197
  • 3
  • 32
  • 40
3

Just move your:

(function (name, func) {...})()

block out of the loop and assign it to a variable, like:

var makeFn = function(name, func){...};

Then in the loop have:

prototype[prop] = makeFn(...)

TheCarver
  • 19,391
  • 25
  • 99
  • 149
Matt Eberts
  • 31
  • 1