html - Javascript setTimeout not working in a for-loop

I have to change the source of an image every second. I have a for loop in which a call a function that has a timeout. I read that here, on stackOverflow, but it doesn't work. Can please someone tell me what can I fix to make it work? I've been struggling with this for much more that I'd like to admit. Thanks.

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <script type="text/javascript">
        function changeImage(k) {
            setTimeout(function(){
              document.getElementById("img").src = k + ".png"; alert(k  );}, 1000);
        }
        function test() {
            for (var k = 1; k <= 3; k++) {
                changeImage(k);

            }
        }
    </script>
</head>

<body>
    <div id="main_img">
        <img id="img" src="">
    </div>
    <input type="button" style="width: 200px" onclick="test()" />
</body>

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <script type="text/javascript">
        function changeImage(k) {
            setTimeout(function(){
              document.getElementById("img").src = k + ".png"; alert(k  );}, 1000);
        }
        function test() {
            for (var k = 1; k <= 3; k++) {
                changeImage(k);

            }
        }
    </script>
</head>

<body>
    <div id="main_img">
        <img id="img" src="http://placehold.it/110x110">
    </div>
    <input type="button" style="width: 200px" onclick="test()" />
</body>

Share Improve this question asked Dec 28, 2013 at 1:29 mkdavor 1344 silver badges16 bronze badges

1 How is it not working? I'd change that alert to console.log as alert will interrupt the script until dismissed. As your script stands (and without the alert) I'd expect all three images to be shown in a probably imperceptible time frame after about a second. You either need to multiply your setTimeout period by k, or use callbacks. – pwdst Commented Dec 28, 2013 at 1:42
You are not changing sources every second, but your changing every source in a second. – Bergi Commented Sep 21, 2014 at 17:32

Add a ment |

6 Answers 6

Sorted by: Reset to default 5

In your code, you set all the timeouts at once. So if you set them all one second from now they all fire one second from now.

You are already passing in the index k so just multiply the time parameter by k.

setTimeout(function(){
  document.getElementById("img").src = k + ".png";
  alert(k);
}, k * 1000);
// ^ added

The problem is that you are creating instances of a timer milliseconds apart. One second later, they all execute milliseconds apart as well. What you need is to execute them at a set interval apart from each other.

You can use a timer using setInterval, which executes the provided function at a given interval. Don't forget to kill-off the timer though, otherwise it will run forever.

Minor optimizations

You can cache the element in a variable so you won't be hitting the DOM that frequently.
Also, I'd avoid the alert(). If you are debugging, use breakpoints in the debugger. If you really want it to be "alert-like", then use console.log and watch the console.
An advantage of setInterval over a recursive setTimeout is that you will not be spawning multiple timers per iteration, but instead, just one timer.

And here's the proposed solution:

var k = 0; 
var image = document.getElementById("img");
var interval = setInterval(function() {

   // Increment or clear when finished. Otherwise, you'll leave the timer running.
   if(k++ < 3) clearInterval(interval);
   image.src = k + ".png";

// Execute block every 1000ms (1 second)
},1000);

Instead of using loop, you can do it like this:

var k = 0; 
var int = setInterval(function() {
   if (k <= 3) k++;
   else { clearInterval(int); }
   document.getElementById("img").src = k + ".png"; 
   alert(k);
}, 1000);

My advice is to use console.log() or alert() to help you debug - it'll make it a LOT more obvious what's going on. For instance, if you put a console.log in your test or setTimeout functions, you'd see that all three images were getting added at the same time.

What I'd remend is to declare your "nextImage" function, then define your setTimeout within that function. That way it'll call itself every second.

Another tip: I assume you want the three images to loop forever, so I added an often used trick with the modulus operator (%) to acplish this.

Have a look:

Working demo: http://jsfiddle/franksvalli/PL63J/2/

(function(){
    var numImages = 3,  // total count of images
        curImage  = 1,  // start with image 1
        $image    = document.getElementById("img"),
        imageBase = "http://placehold.it/110x11";

    function nextImage() {
        $image.src = imageBase + curImage;

        // increment by one, but loop back to 1 if the count exceeds numImages
        curImage = (curImage % numImages) + 1;

        // execute nextImage again in roughly 1 second
        window.setTimeout(nextImage, 1000);
    }

    // initializer.  Hook this into a click event if you need to
    nextImage();
})();

As other folks have said, you probably want to use setInterval, which you can do with some tweaks:

(function(){
    var numImages = 3,  // total count of images
        curImage  = 1,  // start with image 1
        $image    = document.getElementById("img"),
        imageBase = "http://placehold.it/110x11";

    function nextImage() {
        $image.src = imageBase + curImage;

        // increment by one, but loop back to 1 if the count exceeds numImages
        curImage = (curImage % numImages) + 1;
    }

    // initializer.  Hook this into a click event if you need to
    nextImage(); // call function immediately without delay
    window.setInterval(nextImage, 1000);
})();

The problem

setTimeout doesn't stop the program execution but only sets up an event for a callback in 1 second. What that means is that if you setup three setTimeout's inside your for loop, they will execute simultaneously after 1 second.

A solution

Instead of using a for loop, you can use a delayed recursion.

function changeImage(imageIndex) {
    document.getElementById("img").src = imageIndex + ".png"; 
    alert(imageIndex);
}

function myLoop( imageIndex ) {
    if( imageIndex >= 3 ) return;

    changeImage( imageIndex );

    setTimeut( function() { myLoop(imageIndex + 1) }, 1000 );
}

setTimeut( function() { myLoop(0) }, 1000 );

Another solution using setInterval

var interval = null;
var imageIndex = 0;

function changeImage() {
    document.getElementById("img").src = imageIndex + ".png"; 
    alert(imageIndex);

   imageIndex++;
   if( imageIndex === 3 ) clearInterval( interval );
}

interval = setInterval( changeImage , 1000);

Using different delays

function changeImage(imageIndex) {
    document.getElementById("img").src = imageIndex + ".png"; 
    alert(imageIndex);
}

for( var i=0; i < 3; i++) {
    setTimeout( changeImage.bind(window, i), i * 1000 );
}

A groovy one liner( please don't use this, ever! )

(function f(i) { setTimeout( changeImage(i) || f.bind(window, i = (i++)%3), 1000); })(0)

WHY IT DOESN'T WORK?

Because Javascript always passes variables by reference. When your code is waiting on the queue, the variables have already changed.

MY SOLUTION:

Create an array and push whatever codes you want to execute in order of appearance (Place the real value of the variables directly) e.g.:

var launcher = [];
launcher.push('alert("First line of code with variable '+ x +'")');
launcher.push('alert("Second line of code with variable '+ y +'")');
launcher.push('alert("Third line of code with variable '+ z +'")');

Use setInterval instead of setTimeout to execute the codes (You can even change the delay period dynamically) e.g.

var loop = launcher.length;
var i = 0;
var i1 = setInterval(function(){
    eval(launcher[count]);
    count++;
    if(i >= loop) {
        clearInterval(i1);
    }
}, 20);

科技改变生活-雨落星辰 - 所有的伟大,都源于一个勇敢的开始

html - Javascript setTimeout not working in a for-loop - Stack Overflow