Javascript Stack overflow while running setTimeout method + passing variable

Question

I know what the problem I am experiencing is, I am just having a hard time finding a work around. I was wondering if anyone had experienced something like this, and what kinda of solution they implemented.

I have a list system with pending repairs, and I want repairs that are late to blink black and red. It's possible that there are multiple repairs in this list that are late.

here is my function:

function setblink(id) {
   var elm = document.getElementById(id);
    if (elm.color == "red"){
        elm.color = "black";
    }
    else{
        elm.color = "red";
    }
    setTimeout(setblink(id),500);
}

I have an array of "id's" for the items that need to be blinking called repsToBlink.

I get the set blink intervals running for each of these repairs by running the following code, which puts them in a recursive loop.

for(var x in repsToBlink){
setTimeout(setblink(repsToBlink[x]),500);
}

How can I get this code doing the same thing, without causing a stack overflow?

Thanks!

If you want to do this with pure CSS try [this](http://jsfiddle.net/Krr7m/). More info at [MDN](https://developer.mozilla.org/en/CSS/CSS_animations). You will need to add more vendor specific prefixes to get it to work with more browsers and it won't work in older browsers. — seth.miller, Jan 16 '12 at 18:21
I'm not a big fan of a `setTimeout()` for each element. Using a list instead: http://jsfiddle.net/HdCbt/ — Jared Farrish, Jan 16 '12 at 18:48
Dup of [Maximum Call Stack Size Exceeded](http://stackoverflow.com/questions/8731840/), [Why does calling setTimeout with parenthesis not start a new callstack?](http://stackoverflow.com/questions/8058996/). — outis, Jan 16 '12 at 19:19

score 3 · Accepted Answer · edited Jan 16 '12 at 18:21

3

You need to change your setTimeout from

setTimeout(setblink(id),500);

to:

setTimeout(function() { setblink(id) },500);

setTimeout() expects a function to be passed as parameter, whereas you are passing the result of the function.

edited Jan 16 '12 at 18:21

Tomasz Nurkiewicz

334,321
69
703
674

answered Jan 16 '12 at 18:17

techfoobar

65,616
14
114
135

score 2 · Answer 2 · edited May 23 '17 at 10:24

setblink(id) calls the function immediately. A stack overflow is a symptom of immediate, rather than delayed, execution since setTimeout schedules execution for later so the future invocation won't get pushed on the current call stack.

Since setblink takes an argument, wrap it in a nullary anonymous function for lazy evaluation.

function setblink(id) {
   var elm = document.getElementById(id);
    if (elm.color == "red"){
        elm.color = "black";
    }
    else{
        elm.color = "red";
    }
    setTimeout(function () {setblink(id)},500);
}

for (var x in repsToBlink){
    (function (id) {
        setTimeout(function () {setblink(id)},500);
    })(repsToBlink[x]);
}

The code calls for more improvements.

If repsToBlink is an array, you should loop over the integer indices of repsToBlink (for (...;...;...)), not the properties (for ... in). However, if instead you were to use an object with the ids an indices (rather than values), for ... in would be appropriate.

The above fires a separate timer for each id (which could overwhelm the browser). By moving the loop to a function, which becomes the only function to be scheduled, only a single timer is required.

Since you're running a function periodically, setInterval is more appropriate.

Whenever you remove and id from repsToBlink, check whether there are any remaining; if not, cancel the interval.

(function () {
    var repsToBlink, repCount=0, blinkInterval;

    function startBlinking(ids) {
        addRepsToBlink(ids);
        if (! blinkInterval) {
            blinkInterval = setTimeout(blinkAll, 500);
        }
    }

    function addRepsToBlink(ids) {
        for (var i=0; i<ids.length; ++i) {
            addRep(ids[i]);
        }
    }

    function addRep(id) {
        if (! id in repsToBlink) {
            ++repCount;
            repsToBlink[ids[i]] = true;
        }
    }

    function removeRep(id) {
        if (id in repsToBlink) {
            delete repsToBlink[id];
            --repCount;
            if (!repCount) {
                clearInterval(blinkInterval);
                blinkInterval=0;
            }
        }
    }

    function blinkAll() {
        for (id in repsToBlink) {
            blink(id);
        }
    }

    function blink(id) {
        var elm = document.getElementById(id);
        if (elm.color == "red"){
            elm.color = "black";
        } else {
            elm.color = "red";
        }
    }

    window.startBlinking = startBlinking;
    window.addRepsToBlink = addRepsToBlink;
    window.addRep = addRep;
    window.removeRep = removeRep;
})();

score 1 · Answer 3 · answered Jan 16 '12 at 18:19

Your problem is that setTimeout is being called in a global context. That, and you're instantly calling the function.

The interpreter, when it gets to this code:

setTimeout(setblink(id),500);

is calling the setblink function immediately, assuming that that function's return value is what the timeout is supposed to call. This causes a stack overflow, because that is a recursive function.

To fix, this, wrap the function that setTimeout is to call inside a function(){}.

Javascript Stack overflow while running setTimeout method + passing variable

3 Answers3