Javascript - setTimeout Closure Issue

Question

I'm struggling with the code below. I have tried many different ways of doing this but I end up with one of two incorrect results.

for(i = 0; i < result.length; i++) {

    var tmpBlockInfo = {
        size: worldTest.data[0].size,
        xStartPixel :  result[i].x * worldTest.data[0].size,
        yStartPixel : result[i].y * worldTest.data[0].size,
        blockType : (Math.random() * 100 > 10) ? 'path' : 'wall'
    }

    var tmpFunc = function(){
        worldTest.fillBlock(tmpBlockInfo, 157, 152, 124,  255)
    };

    var t = setTimeout(function(){
        tmpFunc()
    } , 500 * i);
}

The problem with the above code is tmpBlockInfo always gets the last result[i].x / result[i].y. So i'm asuming when the timeout runs the function its seeing what result[i].x / result[i].y was left on after the loop (rather than passing it as a "new" variable)

I thought putting it into a function would fix the closure problem, but no luck.

Also tried:

for(i = 0; i < result.length; i++) {

    var tmpBlockInfo = {
        size: worldTest.data[0].size,
        xStartPixel :  result[i].x * worldTest.data[0].size,
        yStartPixel : result[i].y * worldTest.data[0].size,
        blockType : (Math.random() * 100 > 10) ? 'path' : 'wall'
    }

    var t = setTimeout(function(){
        worldTest.fillBlock(tmpBlockInfo, 157, 152, 124,  255)
    } , 10000 * i);
}

With the same results as the first code.

If I do:

for(i=0; i < result.length; i++) {

    var tmpBlockInfo = {
        size: worldTest.data[0].size,
        xStartPixel :  result[i].x * worldTest.data[0].size,
        yStartPixel : result[i].y * worldTest.data[0].size,
        blockType : (Math.random() * 100 > 10) ? 'path' : 'wall'
    }

    setTimeout(function(passBlockInfo) {
        worldTest.fillBlock(tmpBlockInfo, 157, 152, 124,  255) 
    } (tmpBlockInfo), 1000 * i);
}

It does process all the fillBlock functions correctly. BUT it does them all at the same time (e.g. it's not firing them one at a time. It's just doing them after each other but causing blocking (no screen update) and no delay between each one.

Any help with this would be great!

Even though this is a common question, +1 for showing several good attempts to resolve the issue. — , Feb 28 '12 at 16:14
...your last attempt is very close, but you need to *return* a function that runs your code so that `setTimeout` can run it later, and you need to use the `passBlockInfo` parameter instead of the `tmpBlockInfo` — , Feb 28 '12 at 16:17

Zoidberg · Accepted Answer · 2012-02-28T16:31:15.890

7

The reason is it is executing them right away is because you are executing the function in the setTimeout call. What i would do is create another function like so

function MakeTimeoutCall(fn, data, timeout){
    setTimeout(function() {fn.call(null, data);}, timeout);
}

Then in your loop where you call setTimeout, do this

MakeTimeoutCall(
    function(passBlockInfo){
       worldTest.fillBlock(passBlockInfo, 157, 152, 124,  255);
    },
    tmpBlockInfo,
    1000 * i);

(Assuming worldTest is a global object).

That should work. setTimeout is expecting a function to call at the end of the timeout, you are giving your function, but calling it right away. The return value, in this case null, is then given to the timeout. So there is no timeout, everything happens at once.

Just in case my answer is a little complicated given the context, here is a link to a simpler solution in jsfiddle.

edited Feb 28 '12 at 16:31

answered Feb 28 '12 at 16:05

Zoidberg

10,137
2
31
53

P.S. Make sure you scope your i variable with var, or else it will be considered global and there is nothing more dangerous than a global loop variable for causing hard to debug bugs. – Zoidberg Feb 28 '12 at 16:06
Hi, thanks for the reply! I just tried this but I'm getting the same issue as code 1&2 examples above. From what I can tell your point here is just another way of doing what I did in code examples 1&2? – james Feb 28 '12 at 16:11
I forgot the * i in my timeout, let me fix that. – Zoidberg Feb 28 '12 at 16:13
Ahh just changed the scope on the i variable. Thought that was the problem! But no luck! – james Feb 28 '12 at 16:13
I changed you're code already to fix the timeout issue :) MakeTimeoutCall(fn, data, timeout) MakeTimeoutCall( function(passBlockInfo){ worldTest.fillBlock(tmpBlockInfo, 157, 152, 124, 255); }, tmpBlockInfo, 500 * i ); – james Feb 28 '12 at 16:13
There, that is updated. The main difference is that you were calling your function when you set the timeout, here, and in all the other duplicate answers above, the function is not called when the timeout is created. – Zoidberg Feb 28 '12 at 16:14
let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/8315/discussion-between-zoidberg-and-james) – Zoidberg Feb 28 '12 at 16:16
1

Your code won't work because the function being passed to `MakeTimeoutCall` is ignoring the argument passed to the `passBlockInfo` parameter, and is instead using the original `tmpBlockInfo`. I think you may be overcomplicating it a bit. – Feb 28 '12 at 16:23
Thanks for pointing that out, it was a fairly simple oversight on my part. I don't think it is over complicated, I have found this to be a simple way of scoping timeout calls. I have provided a jsfiddle link on my answer to show the solution in a more simple form. – Zoidberg Feb 28 '12 at 16:32

score 4 · Answer 2 · answered Feb 28 '12 at 16:09

4

Try this:

for(i = 0; i < result.length; i++) {
    var tmpBlockInfo = {
        size: worldTest.data[0].size,
        xStartPixel :  result[i].x * worldTest.data[0].size,
        yStartPixel : result[i].y * worldTest.data[0].size,
        blockType : (Math.random() * 100 > 10) ? 'path' : 'wall'
    }
    var t = setTimeout(createFillBlockFn(tmpBlockInfo) , 500 * i);
}

function createFillBlockFn(blockInfo) {
    return function() {
        worldTest.fillBlock(blockInfo, 157, 152, 124,  255)
    }
}

answered Feb 28 '12 at 16:09

bfavaretto

71,580
16
111
150

This is a nice approach. Only downside is JSLint will complain about it, but again, JSLint just likes to hurt people's feelings. – Zoidberg Feb 28 '12 at 16:39
@amnotiam: Will the value of `i` not always be the same inside closure ? That does not seem to take care of that infamous loop problem of closure. – Sarfraz Feb 28 '12 at 16:41
@Sarfraz: From what I can see, the only time `i` is used is outside the handler passed to `setTimeout`, so it is being evaluated immediately inside the loop, instead of later inside the handler. For example, `result[i].x` evaluates `i` immediately to get the item from `result`, and `500 * i` is evaluated immediately to set the duration. – Feb 28 '12 at 16:43
After talking with the James, he has been able to adapt this answer to fit his needs, so if bfavaretto can simply update his answer to fix this small issue I think all will be good. I have also encouraged James to accept this answer as he said he prefers it. – Zoidberg Feb 28 '12 at 16:45
@James make sure you accept one of the answers by clicking the check mark next to the answer. – Zoidberg Feb 28 '12 at 16:45
1

@Zoidberg: Good work man for taking time to fix the issue for OP :) – Sarfraz Feb 28 '12 at 16:48
@Sarfraz thank you as well for taking the time, it was fun. This is why stackoverflow is great. – Zoidberg Feb 28 '12 at 16:49
@Zoidberg, I'll be glad to update my answer, but what's the issue? And why JSLint won't like it? It sure hurts my feelings! :) – bfavaretto Feb 28 '12 at 16:56
@bfvaretto It says you should not create functions inside of other functions. I don't really agree with that rule, I think it helps make javascript a little nicer (as per your answer), but who am I to argue with Douglas Crockford. – Zoidberg Feb 28 '12 at 17:03
1

@Zoidberg, isn't it "[Don't make functions in a loop](http://www.slideshare.net/douglascrockford/3-7687071/72)"? Because Crockford's solution to that is precisely to return a function from another function (see slide 73). BTW: I usually don't use jsLint or minify my scripts (I know...), but [here](http://jsfiddle.net/RDh8b/) is a jsLint-friendly version of my answer. – bfavaretto Feb 28 '12 at 17:44

score 0 · Answer 3 · answered Feb 28 '12 at 16:10

You need a wrapping function to retain scope. This is just one of few ways to do that.

for(i=0; i < result.length; i++) {

    setTimeout(function () {

        var tmpBlockInfo = {
            size: worldTest.data[0].size,
            xStartPixel :  result[i].x * worldTest.data[0].size,
            yStartPixel : result[i].y * worldTest.data[0].size,
            blockType : (Math.random() * 100 > 10) ? 'path' : 'wall'
        };

       return function () {
           worldTest.fillBlock(tmpBlockInfo, 157, 152, 124,  255);
       };
    }(), 1000 * i);
}

score 0 · Answer 4 · answered Feb 28 '12 at 16:10

This code will help you:

for(i=0; i < result.length; i++)
{
    setTimeout(function(){
        {
            size: worldTest.data[0].size,
            xStartPixel :  result[i].x * worldTest.data[0].size,
            yStartPixel : result[i].y * worldTest.data[0].size,
            blockType : (Math.random() * 100 > 10) ? 'path' : 'wall'
        }, 157, 152, 124,  255);
    } , 500 * i);
}

The variables are declared at the top of the function, so you have one var tmpBlockInfo, ont tmpFunc, and those variables you reassigning.

Javascript - setTimeout Closure Issue

4 Answers4