CODEWITHSUNDEEP
javascriptfor-loopsettimeout
joebobs0n
joebobs0n

Reputation: 13

multiple setTimeout calls within a for loop

I'm trying to code the game Simon in HTML/JS and it's all working, except for the part where the game flashes the sequence so you know what the new sequence is. Essentially what I have is:

for(var i in thePattern){
    var obj = document.getElementById(thePattern[i]);
    window.setTimeout(colorON(obj),500);
    window.setTimeout(colorOFF(obj),1000);
}

where colorON and colorOFF are:

function colorON(obj){
    if(obj.id == "red"){
        obj.style.backgroundColor="#ff5555";
    }else if(obj.id == "blue"){
    obj.style.backgroundColor="#5555ff";
    }else if(obj.id == "green"){
    obj.style.backgroundColor="#88ff88";
    }else{
    obj.style.backgroundColor="#ffffaa";
    }
}
function colorOFF(obj){
    if(obj.id == "red"){
        obj.style.backgroundColor="#ff0000";
    }else if(obj.id == "blue"){
        obj.style.backgroundColor="#0000ff";
    }else if(obj.id == "green"){
        obj.style.backgroundColor="#22ff22";
    }else{
        obj.style.backgroundColor="#ffff00";
    }
}

What it seems to be doing is going through the entire for loop and starting all the timers and then then all the timers go off so quickly that the colors don't even seem to flash.

Any ideas? All help is greatly appreciated.

Now it is flashing correctly and the closure is working correctly, but it is flashing all the colors at the same time. I've tried putting the closure within another setTimeout, however, that just creates other problems.

SOLVED thanks for your help guys.

Upvotes: 1

Views: 5493

Answers (2)

epascarello
epascarello

Reputation: 207501

You are calling the function, not assigning a reference to it! So the code runs right away and sets the setTimeout with whatever the function returns.

change 

window.setTimeout(colorON(obj),500);
window.setTimeout(colorOFF(obj),1000);

to

for(var i in thePattern){
    var obj = document.getElementById(thePattern[i]);
    (function(obj) {
        window.setTimeout(function(){colorON(obj);},500);
        window.setTimeout(function(){colorOFF(obj);},1000);
    })(obj);
}

and code showing you how to do with with a switch or an object to get rid of the if/else logic 

function colorON(obj) {
    var color = "";
    switch (obj.id) {
        case "red":
            color = "#ff5555"
            break;
        case "blue":
            color = "#5555ff"
            break;
        case "green":
            color = "#88ff88"
            break;
        default:
            color = "#ffffaa"
    }
    obj.style.background = color;
}

var colorsOff = {
    "red": "#ff0000",
        "blue": "#0000ff",
        "green": "#22ff22",
        "default": "#ffff00"
}



    function colorOFF(obj) {
        var color = colorsOff[obj.id] || colors["default"];
        obj.style.backgroundColor = color;
    }


var thePattern = {
    "one": "red",
        "two": "blue",
        "three": "green"
}


for (var i in thePattern) {
    var obj = document.getElementById(thePattern[i]);
    (function (obj) {
        window.setTimeout(function () {
            colorON(obj);
        }, 500);
        window.setTimeout(function () {
            colorOFF(obj);
        }, 1000);
    })(obj);
}

Example: http://jsfiddle.net/brjgc/

Upvotes: 2

Blender
Blender

Reputation: 298106

You need to pass a function to setTimeout:

window.setTimeout(function() {
    colorON(obj);
},500);

Right now, you're calling colorON(obj) immediately and passing its output to setTimeout, which makes it appear like setTimeout is firing immediately.

obj is also passed by reference, so by the time all of your functions will run, obj will refer to the last element in your loop. To get around that, you have to pass obj by value by shadowing it:

(function(obj) {
    window.setTimeout(function() {
        colorON(obj);
    }, 500);

    window.setTimeout(function() {
        colorOFF(obj);
    }, 1000);
})(obj);

Upvotes: 7

Related Questions