16

I have just started looking at javascript so hopefully this will be something simple. I want to make a slideshow of images that plays automatically. This is very simple, and there are a few tutorials on it but for some reason I haven't been able to get it to work. This is what I have:

var image1 = new Image();
var image2 = new Image(); 
var image3 = new Image();
image1.src = "images/website6.jpg";
image2.src = "images/website7.jpg";
image3.src = "images/sunset.jpg";
var images = new Array(
  "images/website6.jpg",
  "images/website7.jpg",
  "images/sunset.jpg"
);
setTimeout("delay(images,0)",2000);
function delay(arr,num){
  document.slide.src = arr[num % 3];
  var number = num + 1;
  setTimeout("delay(arr,number)",1000);
}

The image I'm trying to change has id slide. And I also have some evidence that it works. What happens is the first image loads. Then the second image loads (which means the original setTimeout call must be working). Then nothing happens. Which to me suggests it's the recursion that isn't working.

I am very familiar with recursion in other languages, so I think it must just be a syntax thing or something, but I can't seem to figure it out. Thanks for any help.

Improve this question
4
  • unquote the parameters in the second setTimeout. that would be my first guess. Commented Apr 5, 2011 at 22:55
  • @Cronco interestingly, if I do that I don't get the second image... which suggests that's what is making the image change once. Also, every example I've seen has quotes, which is why I put them in. Commented Apr 5, 2011 at 22:59
  • The problem with quoting the variables there is that the timer is running the full string "delay(arr,number)" rather than converting the variables to their stored values - that's why unquoting it would work (though you'd still need to quote them like so.... "delay('" + arr + "', '" + number + "'). However, moot point - Pointy's answer is better. I'm just explaining what's happening. Commented Apr 5, 2011 at 23:40
  • 1
    This code isn't recursive. setTimeout registers a handler to be called asynchronously by the event loop when the function call stack (where recursive functions exist) is clear. Commented Jan 15, 2022 at 23:28

3 Answers 3

Reset to default
24

The problem is that when you pass in strings to be evaluated to the setTimeout call, the evaluation will be done (later, when it's time to fire) in the global context. Thus, you're way better off (for a lot of other reasons) passing in actual functions:

setTimeout(function() { delay(images, 0); }, 2000);

function delay(arr, num) {
  document.slide.src = arr[num % 3];
  setTimeout(function() { delay(arr, num + 1); }, 1000);
}

In more modern browsers, you can use the .bind() method for functions to create a function that's pre-bound to something to be used as this:

setTimeout(delay.bind({arr: images, num: 0}), 2000);

function delay() {
  document.slide.src = this.arr[this.num % 3];
  setTimeout(delay.bind({arr: this.arr, num: this.num + 1}), 1000);
}

Six of one, half-dozen of the other, but just as an example that shows there are multiple ways to do things.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Awesome, it works now. I'm still a bit confused as to why my code didn't work though. You say it is evaluated "later, when it's time to fire". When exactly is this, and why does it never reach that part? And also why does using an anonymous function change this? If there is some good documentation that covers this then I'd appreciate a link. Thanks.
The problem is that the string, "delay(arr,number)", needs to be able to get at the variables "arr" and "number" in order for it to make sense. Because of the way string "eval()" works (and now that I think about it, it doesn't matter whether it's at the time of the setTimeout call or later), it's done in a different context than your timeout function itself; it's just the way the language works.
When you use a real function, the interpretation of what the function means happens in the actual scope of your call to "setTimeout()". That function does have access to the local variables there, again because of the way JavaScript works.
I think the first version using closures is better in this case. It avoids bind, which just introduces an extra term (this) into the code. And since setTimeout is called from inside delay(), there is no need for the first setTimeout call, just call delay() and maybe change from repeated setTimeouts to a single setInterval.
@RobG yes I agree, but I included it only for pedagogical reasons :-)
3

I would be very suspicious of the second setTimeout call. I would make it clearer by using an explicit function vs. a string expression

setTimeout(function() { delay(arr, number); }, 1000);
Improve this answer

Comments

1

A bit more compact and passing delay and arguments to setTimeout():

(function delay(arr, num) {
  document.slide.src = arr[num];
  setTimeout(delay, 1000, arr, (num + 1) % 3);
})(images, 0);
Improve this answer

Comments

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.