Scope and Asynchronous JavaScript

Question

I recently ran into an issue at work in which, at least according to my knowledge of JavaScript, I got back an impossible result. I'm hoping someone can explain whats going on here and why the actual results differ from my expected results.

Expected results in console

id: a , x: 1
id: b , x: 1
id: c , x: 1

Actual results in console

id: c , x: 1
id: c , x: 2
id: c , x: 3

Code

function MyClass(id)
{
    var x = 0;

    return function()
    {
        return function()
        {
            x += 1;
            console.log("id: ", id, ", x: ", x);
        }
    }
}


function DoStuff(id)
{
    var q = MyClass(id);
    response_callback = q();
    setTimeout(function(){ response_callback(); }, 50);
}

DoStuff("a");
DoStuff("b");
DoStuff("c");

setTimeout(function(){ response_callback(); }, 50); can be replaced with setTimeout(response_callback, 50) — Cristian Sanchez
– Cristian Sanchez, Commented Aug 2, 2010 at 23:07
@Dan, well it can be, but then you won't get devious bugs, even without the var... — Matthew Flaschen
– Matthew Flaschen, Commented Aug 2, 2010 at 23:10
@Matthew: that sounds like it's the intention to get devious bugs. ;) — Marcel Korpel
– Marcel Korpel, Commented Aug 2, 2010 at 23:41
+1 well written question that sheds interesting light on an easy error to miss. — Gabriel
– Gabriel, Commented Aug 3, 2010 at 2:39

Shog9 · Accepted Answer · 2010-08-02 23:28:06Z

6

response_callback = q();

This. You didn't declare response_callback in any scope, so it's implicitly in the global scope...

Which means you're overwriting it every time you call DoStuff(). You think you're getting three different functions captured and called, but there's only one...

 var response_callback = q(); // should set you back on track

Of course, the way you have this structured right now kinda wastes MyClass's ability to return a function that returns a function. You could actually write:

function DoStuff(id)
{
  var q = MyClass(id);
  // ... do other strange and horrible things with q ...
  setTimeout(q(), 50);
}

...and see the same results without an unnecessary closure.

edited Aug 2, 2010 at 23:28

answered Aug 2, 2010 at 23:02

Shog9

160k36 gold badges237 silver badges242 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

B Keyes Over a year ago

Thank you, its always the simple things that kill you. Only excuse for missing this is that its crunch time, everyone here has been working 12 hour days for a while, and three sets of eyes didn't catch that.

Collectives™ on Stack Overflow

Scope and Asynchronous JavaScript

Expected results in console

Actual results in console

Code

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

Expected results in console

Actual results in console

Code

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related