0

I checked over my script the other day with JSFiddle and got a warning on one of the lines: Don't make functions within a loop.

for (x = 0; x < 10; x++) {

    if (moment(now) > moment(then)) {

        doIt(x); // do it now

    } else {

        timeTillEnd = moment(then) - moment(now);

        setTimeout(function () {

            doIt(x); // do it later

        }, timeTillEnd); // <-- flagged here

    }
}

Why shouldn't I make functions within a loop in Javascript?

Also: Could the usage of a function in the particular situation shown here be problematic?

  • 2
    `x` is not what you think it is. – SLaks Apr 02 '14 at 00:01
  • Because JavaScript has only function scope and every function has a reference to the same variable `x`. – Felix Kling Apr 02 '14 at 00:06
  • Then you probably know the answer to http://stackoverflow.com/questions/22751323/moment-js-useage-of-time-difference-to-set-timeout ; @FelixKling –  Apr 02 '14 at 00:08
  • Just look at http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example, it's all answered there. – Felix Kling Apr 02 '14 at 00:12

2 Answers2

3

What you are trying to do is probably wrong, the x variable might not be what you expect it to be. See the following link:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Closures#Creating_closures_in_loops.3A_A_common_mistake

And they are also relatively expensive to create.

Each function comes with the closure of the variables it uses, that is an unnecessary overhead if you are doing "normal imperative programming" and just want to make the code look clearer by defining inner functions for sub-tasks:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Closures#Performance_considerations

In your case, it seems that you actually need a function with its closure, since you are deferring some computation, but make sure that you do the proper value capture.

fortran
  • 74,053
  • 25
  • 135
  • 175
  • 1
    That's not the (main) reason. Do you have any source for this claim that I could read? – Felix Kling Apr 02 '14 at 00:02
  • http://stackoverflow.com/questions/80802/does-use-of-anonymous-functions-affect-performance – fortran Apr 02 '14 at 00:05
  • 1
    I'm still convinced that that's not the reason why jslint is generating this warning. I mean, it should be clear that one function uses less memory than 1000 functions. But the same goes for arrays, objects, strings, numbers, any kind of data. – Felix Kling Apr 02 '14 at 00:08
  • there might be the other reason about the capture of the `x` value – fortran Apr 02 '14 at 00:10
  • This answer (wrt. performance) does not match reality. There is no reason why methods-in-a-loop *are* more expensive, and jsperfs I've done have shown that it is "well optimized". While a closure is "created", here is the kicker: it is always the *same outer scope* that is being closed-over in the lexical chain for the newly created functions. – user2864740 Apr 02 '14 at 00:12
  • 1
    (Okay, the new reason at the top of this answer makes more sense to me. However, an IIFE would still be "making a function" in a loop.) – user2864740 Apr 02 '14 at 00:15
  • @user2864740 creating functions is an expensive operation anyway – fortran Apr 02 '14 at 00:16
  • 1
    Maybe you find this interesting regarding the memory footprint of functions: https://groups.google.com/forum/#!topic/v8-users/BbvL5qFG_uc. – Felix Kling Apr 02 '14 at 00:16
  • @fortran No, it *does not need to be*. The function body is immutable. That is, each function-object may utilize the exact same body which can be pulled "out" of the loop. As such, the only thing that needs to be "created" is the thin-shell function-object and as *only one outer scope is bound*, that can likewise be treated as though it did not occur in the loop. – user2864740 Apr 02 '14 at 00:16
  • @user2864740 https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Closures#Performance_considerations – fortran Apr 02 '14 at 00:17
  • @fortran Not related to this case which can be and *is* well optimized (albeit inconsistently) across browsers. – user2864740 Apr 02 '14 at 00:17
  • @fortran: As I said, that counts for any value that is not necessary. It doesn't mean that the creating of a function is more expensive than the creation of any other object. – Felix Kling Apr 02 '14 at 00:18
  • @FelixKling some objects are heavier than others, functions are not exactly lightweight (like a number might be)... – fortran Apr 02 '14 at 00:20
  • @fortran *Why* aren't a function-object implemented in an *efficient lightweight manner*? (They are, run-time engine dependent of course, as the they are very prevalent to many aspects of JavaScript. The fact that they might not be implemented as *immediate values* is irrelevant to all the other optimizations that modern JS run-times and JITs perform.) – user2864740 Apr 02 '14 at 00:21
  • 1
    If you can refer me to some technical articles how engines implement functions, I'd be happy. I agree that it "feels" like the creation of functions could be more expensive than, say, a plain object, but JS engines also made many advances in recent years, and I just don't believe such a statement without any backup. – Felix Kling Apr 02 '14 at 00:23
0

Because it can lead to unexpected closure behaviour (the captured variable will have the value assigned in the last iteration of the loop). You will also get a new instance of the function for each loop which is wasteful of resources.

Modern browsers take a third argument for setTimeout which is the argument to the function. See here. This also gets rid of the problems with closures.

acarlon
  • 16,764
  • 7
  • 75
  • 94