HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

A Nightmare on setTimeout Street

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
settimeoutstreetnightmare

Problem

I have script which plays audio, shows/hides elements:

var audio = new Audio("audio/test.mp3");

    audio.addEventListener("ended", function(){ 
             element.fadeIn(0);
             setTimeout(function() {
                element2.fadeIn(0);
                setTimeout(function() {
                    element3.fadeIn();
                    sprite.play(); // a one lib for playing sprites
                },1000)
             },1000)
        });

// this is just event and we look for the last frame
sprite.on("frame", function(){
        if(this.frame === this.frames - 1)
        {
            this.pause();           
            setTimeout(function() {
                //here we show some another elements and play another audio
            },1000)
        }
    }); 

audio.play();


As you can see there are a lot of setTimeout and spaghetti code. This is very short version, I have more than 5 audio files and a lot of elements which I have to show and hide, and, it's very difficult to understand the order audio files playing and showing/hiding elements in the end. May be you know how to organize this flow, some libraries? I am a noob at promises, is it good idea here?

Solution

To generate those nested, nearly identical setTimeout calls, instead of coding them all manually you could have a recursive function that builds and returns a function for you. Something like this would work:

// Accepts an array of elements and a final callback function
// Returns a function to show all elements and execute the callback
function getFunctionToShowElements(elements, callback, currentIndex) {
    var i = typeof currentIndex === "undefined" ? 0 : currentIndex;
    if (i === elements.length) {
        // after the final element, call the callback
        return function () {
            callback();
        };
    } else {
        // for any preceding element, show the element and call a timeout
        var nextMethod = getFunctionToShowElements(elements, callback, i + 1);
        return function () {
            elements[i].fadeIn();
            setTimeout(nextMethod, 1000);
        };
    }
}


Then you could set up a function for creating audio objects and attaching the event listener to fade in the given elements.

function createAudio(filename,elements){
    var audio = new Audio(filename);
    audio.addEventListener("ended",function(){
        getFunctionToShowElements(elements,function(){sprite.play();})(); // get the function and execute it immediately
    });
    return audio;
}


At this point you'll be able to create an audio object with the desired event handler like so:

var audio = createAudio("audio/test.mp3",[element1,element2,element3]);


That's just to give you an idea of how you could go about cleaning it up.

There are other improvements you'd want to make in your final code, such as using a variable for the timeout delay, and wrapping up the relevant code sections in closures as necessary. We'd need to see the full code to identify other areas for improvement.

Proof that the concept works:



var els = document.querySelectorAll(".showme");
var sprite = {
play: function () {
alert("sprite.play");
}
};
createAudio("audio/test.mp3", els);

function createAudio(filename, elements) {
var audio = document.querySelector("button"); //new Audio(filename);
audio.addEventListener("click", function () {
getFunctionToShowElements(elements, function () {
sprite.play();
})();
});
}

function getFunctionToShowElements(elements, callback, currentIndex) {
var i = typeof currentIndex == "undefined" ? 0 : currentIndex;
if (i === elements.length) {

return function () {
callback();
};
} else {

var nextMethod = getFunctionToShowElements(elements, callback, i + 1);
return function () {
elements[i].style.display = "inherit";
setTimeout(nextMethod, 1000);
};
}
}

.showme {
background-color:red;
display:none;
}

Click me
Number 1
Number 2
Number 3
Number 4

Code Snippets

// Accepts an array of elements and a final callback function
// Returns a function to show all elements and execute the callback
function getFunctionToShowElements(elements, callback, currentIndex) {
    var i = typeof currentIndex === "undefined" ? 0 : currentIndex;
    if (i === elements.length) {
        // after the final element, call the callback
        return function () {
            callback();
        };
    } else {
        // for any preceding element, show the element and call a timeout
        var nextMethod = getFunctionToShowElements(elements, callback, i + 1);
        return function () {
            elements[i].fadeIn();
            setTimeout(nextMethod, 1000);
        };
    }
}
function createAudio(filename,elements){
    var audio = new Audio(filename);
    audio.addEventListener("ended",function(){
        getFunctionToShowElements(elements,function(){sprite.play();})(); // get the function and execute it immediately
    });
    return audio;
}
var audio = createAudio("audio/test.mp3",[element1,element2,element3]);

Context

StackExchange Code Review Q#92561, answer score: 9

Revisions (0)

No revisions yet.