Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

changing the scope of an anonymous function on a setTimeout causes a weird warning

this has interested me purely as research and personal development. i have a namespaced set of functions / variables.

within 1 function I need to call another through setTimeout but keeping the scope to 'this'. i am struggling with this a little, can't seem to bind it for when the setTimeout runs.

var foo = {
    ads: ["foo","bar"],
    timeDelay: 3,
    loadAds: function() {
        var al = this.ads.length;
            if (!al)
                return; // no ads

            for(var i = 0; i < al; i++) {
                setTimeout(function() {
                    this.scrollAd(this.ads[i]);
                }.apply(this), this.timeDelay * 1000);
            }
        },
        scrollAd: function(adBlock) {
            console.log(adBlock);

        }
    };
};

the .apply(this) DOES change the scope as the console.log outputs the right object back, but it runs the function immediately and then the exception/warning comes up as the callback remains empty:

useless setTimeout call (missing quotes around argument?)

is there an elegant way of doing this at all? i know i could do

var _this = this;

and reference _this in the anon callback. for example, in mootools i'd use .bind(this) instead...

and no, as this involves animating, i don't want to use " " around the string as it will need to be eval'd and would impact performance...

like image 865
Dimitar Christoff Avatar asked Dec 23 '22 07:12

Dimitar Christoff


2 Answers

for(var i = 0; i < al; i++) {
    setTimeout(function() {
        this.scrollAd(this.ads[i]);
    }.apply(this), this.timeDelay * 1000);
}

apply doesn't bind a function, it calls it. So you execute the scroll straight away and then pass its return value (undefined) to setTimeout, which is ineffective.

You probably meant to use a closure like this over this and the loop variable (which must be closed or it will be the same, post-loop value for every timeout):

for(var i = 0; i < al; i++) {
    setTimeout(function(that, j) {
        return function() {
            that.scrollAd(that.ads[j]);
        };
    }(this, i), this.timeDelay * 1000);
}

However you may prefer to use the new ECMAScript Fifth Edition function binding feature, which has a much more compact syntax:

for (var i= 0; i<al; i++)
    setTimeout(this.scrollAd.bind(this, this.ads[i]), this.timeDelay*1000);

(There's an implementation of function.bind for browsers that don't have have it natively at the bottom of this answer.)

like image 179
bobince Avatar answered Apr 18 '23 06:04

bobince


From what I know you should indeed use something like this:

var self = this;
setTimeout(function(){self.scrollAd(ad);}, this.timeDelay * 1000);

But if you badly want to use .apply(), then do it like this:

var self = this;
setTimeout(function(){
    function(){
    }.apply(self);
}, this.timeDelay * 1000);

Also note that if you run this inside a for loop and use i's value inside a function that is run in timer, then your function will always run with the last value of i (i.e. i == al). In order to fix that, you'll need to make a closure with each value of i separately.

So taking your code and making it work it should look like this:

var foo = {
    ads: ["foo","bar"],
    timeDelay: 3,
    loadAds: function() {
        function runTimed(o, fn, args, time)
        {
            setTimeout(function(){ fn.apply(o, args); }, time);
        }
        var al = this.ads.length;
            if (!al)
                return; // no ads

            for(var i = 0; i < al; i++) {
                runTimed(this, this.scrollAd, this.ads[i], this.timeDelay*1000);
            }
        },
        scrollAd: function(adBlock) {
            console.log(adBlock);
        }
    };
};

Note: I haven't run this code so it may contain some mistakes.

Also if I were you, I'd use the data from object and don't pass it to the scrollAd (i is enough).

like image 23
inkredibl Avatar answered Apr 18 '23 07:04

inkredibl