CODEWITHSUNDEEP
javascriptloops
HandiworkNYC.com
HandiworkNYC.com

Reputation: 11104

Javascript passing parameters to a setTimeout function inside of a loop

I'm trying to execute a setTimeout() function inside of a for loop in Javascript, but I am getting the error "shape is undefined", even though it is defined, and I'm passing that as a parameter in the function within the setTimeout() call. The function works just fine if I delete the setTimeout enclosure.

Why am I getting this error and how can I fix it?

Thanks!

function fadeShapes(layer, movement, opacity, speed) {
    var shapes = layer.getChildren();

    for(var n = 0; n < shapes.length; n++) {
        var shape = layer.getChildren()[n];
        setTimeout(function(shape){
            shape.transitionTo({
                alpha: opacity,
                duration: speed
            }); 
        }, 100);                
    }
}

Upvotes: 4

Views: 3625

Answers (5)

Halcyon
Halcyon

Reputation: 57709

This is a common closure problem, here is the fixed code:

function fadeShapes(layer, movement, opacity, speed) {
    var shapes = layer.getChildren();

    for(var n = 0; n < shapes.length; n++) {
        var shape = layer.getChildren()[n];
        setTimeout((function (bound_shape) {
            return function() {  // return function!
                bound_shape.transitionTo({
                    alpha: opacity,
                    duration: speed
                }); 
            };
        }(shape)) // immediate execution and binding
        , 100);                
    }
}

What happens in your code is that the for loop will run and n functions will be scheduled from execution in 100ms, but the value of shape changes! So when your callback gets called shape is the value of shapes[length-1] (the last shape).

To fix it, you must 'close' the value using a closure. In this case a function that binds on the value of shape and returns the function you want executed in 100ms.

Upvotes: 1

Dennis
Dennis

Reputation: 32598

JavaScript does not have block scope so all of your timeout functions are pointing at the same variable shape, which after the loop finishes points to an undefined index of your array. You can use an anonymous function to emulate the scope you're looking for:

for(var n = 0; n < shapes.length; n++) {
    var shape = shapes[n]; //Use shapes so you aren't invoking the function over and over
    setTimeout((function(s){
        return function() { //rename shape to s in the new scope.
            s.transitionTo({
                alpha: opacity,
                duration: speed
            });
        };
    })(shape), 100);   
}

As you could tell by my issues getting the brackets matched up, this can be a little tricky. This can be cleaned up with ES5's Array.forEach:

layer.getChildren().forEach(function(shape) { //each element of the array gets passed individually to the function
    setTimeout(function(shape){
        shape.transitionTo({
            alpha: opacity,
            duration: speed
        }); 
    }, 100);                
});

forEach is built into modern browsers but can be shimmed in Internet Explorer older browsers.

Upvotes: 12

Ikrom
Ikrom

Reputation: 5103

Try:

function fadeShapes(layer, movement, opacity, speed) {
    var shapes = layer.getChildren();

    for(var n = 0; n < shapes.length; n++) {
        var shape = layer.getChildren()[n];
        (function(sh) {
            setTimeout(function(){
                sh.transitionTo({
                    alpha: opacity,
                    duration: speed
                }); 
            }, 100); 
        })(shape);
    }
}

Upvotes: 0

Greg Rozmarynowycz
Greg Rozmarynowycz

Reputation: 2085

If you want the the setTimeout calls to be executed 100ms apart, then you would just add 100ms to each set Timeout call:

function fadeShapes(layer, movement, opacity, speed) {
    var shapes = layer.getChildren();

    for(var n = 0; n < shapes.length; n++) {
        var shape = shapes[n];
        setTimeout((function(local_shape){
            return function(){
                local_shape.transitionTo({
                    alpha: opacity,
                    duration: speed
                });
             } 
        })(shape), 100 + n*100);                
    }
}

Upvotes: 0

Halcyon
Halcyon

Reputation: 57709

Here is the code for an iterator that respects the 100ms delay. (untested)

function iterate(array, timeout, callback) {
    var n = 0, length = array.length;
    function step() {
        callback(array[n]);
        n += 1;
        if (n < length) {   // are there more elements?
            setTimeout(step, timeout);
        }
    }
    setTimeout(step, timeout);  // start
}

function fadeShapes(layer, movement /* unused> */, opacity, speed) {
    var shapes = layer.getChildren();
    iterate(shapes, 100, function (shape) {
        shape.transitionTo({
            alpha: opacity,
            duration: speed
        }); 
    });
}

Note that by using the iterate function in this way the closure issues are solved as well.

Upvotes: 0

Related Questions