Fede
Fede

Reputation: 53

Is this a JavaScript closure bug?

Can someone explain me why the code below doesn't print the numbers from 0 to 2 on this.closure unless I do new crazyClosure(i) or crazyClosure.call({}, i)?

var crazyClosure = function (count) {
    var self = this;

    function closure () {
        console.log("closure", count);
    };

    this.closure = function () {
        console.log("this.closure", count);
    };

    console.log("init", count);
    setTimeout(function () {
        console.log('');
        console.log("call", count);
        closure();
        self.closure();
    }, 0);
}

for (var i = 0; i < 3; i++) {
    crazyClosure(i);
}

Is like if the closure would be attached to this instead of to the crazyClosure function itself.

I would expect to see:

init 0
init 1
init 2
call 0
closure 0
this.closure 0
call 1
closure 1
this.closure 1
call 2
closure 2
this.closure 2

but I get

init 0
init 1
init 2
call 0
closure 0
this.closure 2
call 1
closure 1
this.closure 2
call 2
closure 2
this.closure 2

Adding another example according to answers. In this example I have 2 functions on this which is window that get overridden every time I call crazyClosure though I get the result I was expecting when calling expected but I get the result you guys say I should get when calling unexpected. According to your explanation I still don't understand how that can happen.

function crazyClosure (count) {
    var self = this;

    this.expected = function () {
        console.log("this.expected", count);
        self.unexpected();
    };

    this.unexpected = function () {
        console.log("this.unexpected", count);
    };

    console.log("init", count);
    setTimeout(self.expected, 10);
}

for (var i = 0; i < 3; i++) {
    crazyClosure(i);
}

I get:

init 0
init 1
init 2
this.expected 0
this.unexpected 2
this.expected 1
this.unexpected 2
this.expected 2
this.unexpected 2

I need to understand this!!! Thanks

Upvotes: 0

Views: 372

Answers (2)

6502
6502

Reputation: 114599

You should try to separate the concept of closure from the concept of Javascript object.

They are totally unrelated and mixing them while learning can bring in more confusion than needed.

In your code the problem is that crazyClosure seems like a constructor, but is called as a function so it will not receive a this object to work on and thus will work with this that is set to window.

The result is that self is also going to be tied to window and the call self.closure() will invoke last closure your loop created. All three timeout events will invoke last closure because Javascript will not execute a timeout until your loop terminates.

Please also do yourself a favour and leave immediately the "Javascript closures are broken" attitude. If your first thought is that other people's code is broken you are not going to go any far in programming. Please don't take it personally, and don't think it's a rant about how politely you ask a question. The mental habit of thinking that your code is correct and other's people code (a library, the compiler, the OS) is wrong is a true killer... don't fall into that trap.

Edit

The second example is more convoluted but still the resulting output is what a Javascript implementation should do.

One each call as said before this will be window because the new operator was not used when calling crazyClosure.

The two "methods" expected and unexpected will end up being global functions and you can check this because after the execution you will be able to call expected() directly from outside crazyClosure.

However while inside crazyClosure the value passed to setTimeout will be different each time because the global expected will have been just overwritten with a new closure that can access the local count variable. This expected global variable will be overwritten later, but when the value has been already passed to setTimeout and the Javascript runtime has stored it away to call it 10 milliseconds later.

All those three closures however in their code call self.unexpected, and when this happens this global has been already overwritten three times, so all the three different closures "expected" will call the same closure "unexpected" (the last one) because when the timeout triggers it's what is currently stored in window.unexpected.

Upvotes: 3

Denys S&#233;guret
Denys S&#233;guret

Reputation: 382514

If you don't use new, this at the start of the crazyClosure function is the enclosing context, not a new object. So all the calls end calling the closure function on the same self (which is window if your code isn't embedded).

Each call to

this.closure = function () {
    console.log("this.closure", count);
};

is equivalent to

window.closure = function () {
    console.log("this.closure", count);
};

and thus replaces the precedent function. You end up with only one window.closure function, embedding the last value, and that's this unique function you call three times with setTimeout.

Upvotes: 1

Related Questions