CODEWITHSUNDEEP
javascript
PawelRoman
PawelRoman

Reputation: 6262

javascript closures in loops

//I have the following function:
    function handle_message(msg)
    {
        //do work
        console.log('some work: '+msg.val);
        //call next message
        msg.next();
    }

//And array of message objects:
var msgs = [ {val : 'first msg'}, { val : 'second msg'}, { val : 'third msg'}];

//I link messages by setting next parameter in a way that it calls handle_message for the next msg in the list. Last one displays alert message.
msgs[2].next = function() {alert('done!')};
msgs[1].next = function() {handle_message(msgs[2]);};
msgs[0].next = function() {handle_message(msgs[1]);};

//Start the message handle "chain". It works!
handle_message(msgs[0]);

//======== Now I do exactly the same thing but I link messages using the for loop:

for (var i=msgs.length-1; i>=0; i--)
{
    if (i==msgs.length-1)
    {
        msgs[i].next = function() {alert('done!');};
    }
    else
    {
        msgs[i].next = function() {handle_message(msgs[i+1]);};
    }
}

//Start the message handling chain. It fails! It goes into infinite recursion (second message calls itself)
handle_message(msgs[0]);

Can sombody explain why it happens? Or maybe an alternative to this pattern? My case is this: I receive an array with messages and I have to handle them in order, one ofter another SYNCHRONOUSLY. The problem is some of the messages require firing a series of animations (jqwuery animate() which is async) and the following messages cannot be handled until the last animation is finished. Since there is no sleep() in javascript I was trying to use such pattern where the message calls the next one after it is finished (in case of animations I simply pass the 'next' function pointer to animate's "complete" callback). Anyway, I wanted to build this 'chain' dynamically but discovered this strange (?) behaviour.

Upvotes: 1

Views: 125

Answers (3)

Šime Vidas
Šime Vidas

Reputation: 185873

You need a closure to make it work:

function handle_message( msg ) {
    console.log( 'some work: ' + msg.val );
    msg.next();
}

var msgs = [{val :'first msg'},{val:'second msg'},{val:'third msg'}];

for ( var i = msgs.length - 1; i >= 0; i-- ) {
    (function(i) {
        if ( i == msgs.length - 1 ) {
            msgs[i].next = function() { alert( 'done!' ); };
        } else {
            msgs[i].next = function() { handle_message( msgs[i + 1] ); };
        }
    })(i);
}

handle_message( msgs[0] );

Live demo: http://jsfiddle.net/simevidas/3CDdn/

Explanation:

The problem is with this function expression:

function() { handle_message( msgs[i + 1] ); }

This function has a live reference to the i variable. When this function is called, the for loop has long ended and the value of i is -1. If you want to capture the current value of i (the value during the iteration), you need to an additional wrapper function. This function captures the current value of i permanently (as an argument).

Upvotes: 3

Álvaro González
Álvaro González

Reputation: 146330

I think the problem is that i doesn't have the value you think it has:

// i is defined here:
for (var i=msgs.length-1; i>=0; i--)
{
    if (i==msgs.length-1)
    {
        msgs[i].next = function() {alert('done!');};
    }
    else
    {
        msgs[i].next = function() {
            // when this line gets executed, the outer loop is long finished
            // thus i equals -1
            handle_message(msgs[i+1]);
        };
    }
}

See point #5 Closures in loops at http://blog.tuenti.com/dev/top-13-javascript-mistakes/

Upvotes: 2

Jeff Foster
Jeff Foster

Reputation: 44696

Think about the values you are capturing in the closure.

msgs[i].next = function() {handle_message(msgs[i+1]);};

This captures the value of i, but it changes the next iteration so you get an infinite loop.

By the end of the loop i is -1 so i+1 is going just going to be the same message over and over again.

Upvotes: 1

Related Questions