0
\$\begingroup\$

I wrote a function to make a image slideshow in javascript using for() loop, setTimeout() and setInterval(), but I have doubts that is this a right approach to use a for() loop and setTimeout() together?

Is there a better way to do it...?

var myImage = document.querySelectorAll(".banner");
var arrayImage = ["https://via.placeholder.com/150x150/f00/fff?text=Slider1", "https://via.placeholder.com/150x150/f0f/fff?text=Slider2", "https://via.placeholder.com/150x150/00f/fff?text=Slider3"];
var imageIndex = 0;

function slideChange() {
  for (var i = 0; i < arrayImage.length; i++) {
    (function(i) {
      setTimeout(function() {
        myImage[i].setAttribute("src", "");
      }, 2000 * (i + 1));
      setTimeout(function() {
        myImage[i].setAttribute("src", arrayImage[i]);
      }, 2000 * i);
    })(i)
  }
}
slideChange();
setInterval(function() {
  slideChange();
}, (2000 * arrayImage.length))
.banner {
  display: block;
}
<img class="banner">
<img class="banner">
<img class="banner">

\$\endgroup\$
  • \$\begingroup\$ Based on your demo, it doesn't appear that this works correctly as a slideshow. The timing is off — there is sometimes more or fewer than one visible image. Before proceeding with a review, you'll either have to fix your code, or change the description of what the intended behavior is. \$\endgroup\$ – 200_success Mar 1 '18 at 6:02
  • \$\begingroup\$ @200_success sorry to say but I think a slideshow is something like the slide changing after some time....is it not...? \$\endgroup\$ – Bhuwan Mar 1 '18 at 6:28
  • \$\begingroup\$ Yes, but the timing of the appearances / disappearances becomes desynchronized after a while. I've reopened the question so that that issue may be addressed in an answer. \$\endgroup\$ – 200_success Mar 1 '18 at 6:31
  • \$\begingroup\$ works fine for me in firefox \$\endgroup\$ – I wrestled a bear once. Mar 2 '18 at 13:24
1
\$\begingroup\$

There's really no purpose in having 3 banner images if you're going to add and remove the image source from each one. You might as well just have a single image and change the source of it on each iteration.

When you do that trick with setTimeout, where your timeout is the index of the array multiplied by an offset, you are effectively creating a new thread for every iteration, which can be memory intensive when you have a lot of images. A better approach would be to use recursion.

Consider this slowLoop function.

/**
 * Execute the loopBody function once for each item in the items array, 
 * waiting for the done function (which is passed into the loopBody function)
 * to be called before proceeding to the next item in the array.
 * @param {Array} items - The array of items to iterate through
 * @param {Function} loopBody - A function to execute on each item in the array.
 *      This function is passed 3 arguments - 
 *          1. The item in the current iteration,
 *          2. The index of the item in the array,
 *          3. A function to be called when the iteration may continue.
 * @returns {Promise} - A promise that is resolved when all the items in the 
 *      in the array have been iterated through.
 */
function slowLoop(items, loopBody) {
    return new Promise(f => {
        let done = arguments[2] || f;
        let idx = arguments[3] || 0;
        let cb = items[idx + 1] ? () => slowLoop(items, loopBody, done, idx + 1) : done;
        loopBody(items[idx], idx, cb);
    });
}

Using this you're able to keep the entire script on a single thread.

var myImage = document.querySelector("#banner");

var arrayImage = [
  "https://via.placeholder.com/150x150/f00/fff?text=Slider1",
  "https://via.placeholder.com/150x150/f0f/fff?text=Slider2",
  "https://via.placeholder.com/150x150/00f/fff?text=Slider3"
];

function slowLoop(items, loopBody) {
    return new Promise(f => {
        let done = arguments[2] || f;
        let idx = arguments[3] || 0;
        let cb = items[idx + 1] ? () => slowLoop(items, loopBody, done, idx + 1) : done;
        loopBody(items[idx], idx, cb);
    });
}

function slideChange(){
  slowLoop(arrayImage, function(img, idx, done){
    myImage.setAttribute("src", img);
    setTimeout(done, 2000);
  }).then(slideChange);
}

slideChange();
.banner {
  display: block;
}
<img id="banner">

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.