3

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>

2
  • 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. Commented Dec 28, 2013 at 1:42
  • You are not changing sources every second, but your changing every source in a second. Commented Sep 21, 2014 at 17:32

6 Answers 6

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
Sign up to request clarification or add additional context in comments.

Comments

2

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

1 Comment

If you really need to use a ternary if, you can even use it like this. (k++ < 3) ? clearInterval(interval);. Or even this if( k++ < 3 ) clearInterval(interval);. but you deserve a +1
1

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

Comments

1

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

Comments

1

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)

Comments

1

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

1 Comment

Contrary to what most beginners think of 'eval();', it doesn't actually expose you to any security risks - other than the ones you've created yourself API-wise (They have nothing to do with Javascript.) And it does perform quicker compared to some other solutions which creates multiple setTimeout instances.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.