CODEWITHSUNDEEP
javascripthtmlfunctionsettimeout
mkdavor
mkdavor

Reputation: 134

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="http://placehold.it/110x110">
    </div>
    <input type="button" style="width: 200px" onclick="test()" />
</body>

Upvotes: 3

Views: 1976

Answers (6)

RobbY
RobbY

Reputation: 11

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);

Upvotes: 1

Igor Šarčević
Igor Šarčević

Reputation: 3551

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)

Upvotes: 1

Joseph
Joseph

Reputation: 119847

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);

Upvotes: 2

David Calhoun
David Calhoun

Reputation: 8581

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 recommend 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 accomplish this.

Have a look:

Working demo: http://jsfiddle.net/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);
})();

Upvotes: 1

Ringo
Ringo

Reputation: 3965

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);

Upvotes: 1

Alex Wayne
Alex Wayne

Reputation: 187034

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

Upvotes: 5

Related Questions