CODEWITHSUNDEEP
javascripthtmldynamictimeoutsettimeout
Flawed_Logicc
Flawed_Logicc

Reputation: 145

Javascript Stack overflow while running setTimeout method + passing variable

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!

Upvotes: 0

Views: 295

Answers (3)

outis
outis

Reputation: 77420

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;
})();

Upvotes: 2

techfoobar
techfoobar

Reputation: 66693

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.

Upvotes: 3

Jeffrey Sweeney
Jeffrey Sweeney

Reputation: 6124

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(){}.

Upvotes: 1

Related Questions