2

I am having a problem with a classic javascript "issue"!

I want to create an array of functions that will each be set to run after different lenghs of time (each one second apart).

However the following code is creating functions that all have the timeout set to the last value of timeout.

Any help to solve this would be wonderful! Thanks!

    var timeout = 0;
    var myfunctions = []

    urls.forEach(function(url){

      var newFunction = function(callback){
        (function (timeout){
          setTimeout(function(){
            getTransactions(url, function(err, data){
              callback(err, data)
            })
          },timeout)

        })(timeout)

      }
      timeout = timeout + 1000
      myfunctions.push(newFunction)
    })

The reason each function has a callback is that I that plan to run them all using:

    async.parallel(myfunctions,
      function(err, results) {
        if(err) {
          console.log(err.message)
        }
        else{
          console.log(results)
        }
      })
  • Hi Steven Winston, can you provide a running example of the issue using code pen or jsfiddle etc or stackoverflows builtin tools to test your js?? – Irfan Anwar Aug 28 '16 at 17:04
  • 2
    **Not** a duplicate of [*JavaScript closure inside loops – simple practical example*](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) *(for once!)*, as the OP is actually trying to apply that technique, just getting it slightly wrong. – T.J. Crowder Aug 28 '16 at 17:06

1 Answers1

3

You've just misapplied the usual pattern; you're doing it too far in, waiting to call the inner IIFE until your function is called, at which point timeout is already updated.

But there's no need for the IIFE at all, you can use the closure of the callback to forEach:

var timeout = 0;
var myfunctions = []

urls.forEach(function(url) {
    var thisTimeout = timeout;
    var newFunction = function(callback) {
        setTimeout(function() {
            getTransactions(url, function(err, data) {
                callback(err, data)
            })
        }, thisTimeout)
    };
    timeout = timeout + 1000
    myfunctions.push(newFunction)
});

Live Example:

var timeout = 0;
var myfunctions = [];

var urls = ["first", "second", "third"];

urls.forEach(function(url) {
    var thisTimeout = timeout;
    var newFunction = function(callback) {
        console.log("url = " + url + ", thisTimeout = " + thisTimeout);
        /*
        setTimeout(function() {
            getTransactions(url, function(err, data) {
                callback(err, data)
            })
        }, thisTimeout)
        */
    };
    timeout = timeout + 1000
    myfunctions.push(newFunction)
});

myfunctions.forEach(function(f) {
  f();
});
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • Because var has a function scope so it won't work as your expectation anyway. For ECMA6, you can use `let thisTimeout = timeout;` instead. – Daniel Tran Aug 28 '16 at 17:07
  • 3
    @DanielTran: The above does work. The `var`/`let` difference doesn't matter here, as we have `forEach`, not `for`. – T.J. Crowder Aug 28 '16 at 17:07