patternjavascriptMinor
A Nightmare on setTimeout Street
Viewed 0 times
settimeoutstreetnightmare
Problem
I have script which plays audio, shows/hides elements:
As you can see there are a lot of
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
Then you could set up a function for creating
At this point you'll be able to create an
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:
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 4Code 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.