fade in / out banner

Question

I am trying to create a fade in / out banner on my website. I have an array of id's. I want to loop through the array thus showing first after a time limit and hiding then going to the second array to repeat the same proceedure.

my code....

$(Document).ready(function () {
var images = new Array();
images[0] = "#bannerImageOne";
images[1] = "#bannerImageTwo";
$('#homeCarousel img').hide();

for (var i in images) {
    setTimeout(fadeInOut(i, images), 5000);
    //alert(i);
}
});


function fadeInOut(i, images) {
$(images[i]).fadeIn("slow").delay(2000).fadeOut();
//alert(images[i]);
}

my problem is that only the first banner is displaying and in the alert only the first id is showing. Is there a better solution to this problem?

thanks....

Answer 1

Overview

This answer comes in two parts: Help with the actual code you posted, and then a recommendation for another way to do it

Help with the code you posted

This code:

for (var i in images) {
    setTimeout(fadeInOut(i, images), 5000);
    //alert(i);
}

...has one major and one minor issue:

1. The major issue: You're calling the fadeInOut function and passing its return value into setTimeout, just like any other time you do function1(function2(arg, arg2));. Instead, change the setTimeout line to:

   setTimeout(createFader(i, images), 5000);
   

   ...and add a createFader function that looks like this:

   function createFader(index, array) {
       return function() {
           fadeInOut(index, array);
       };
   }
   

   The createFader function creates a function that, when called, will call fadeInOut with the arguments you passed into it. You cannot just do this:

   setTimeout(function() {  // <== WRONG, WILL NOT WORK
       fadeInOut(i, images);
   }, 5000);
   

   ...because the anonymous function there (which is a closure) will get an enduring reference to i and images, and so when the timeout occurs, it'll use whatever their values are then, not what they were when you created the function. More: Closures are not complicated

   (Don't use strings with setTimeout as another answer suggests suggested.)

2. for..in is for looping through the enumerable property names of an object, not for looping through array indexes. In trivial environments, it mostly works, but it's a bad habit that will bite you if you don't understand exactly what you're doing. More here: Myths and realities of for..in If you're using jQuery, and if it's okay to have the overhead of a couple of function calls, $.each is great for looping through arrays (except it's not ideal for sparse arrays, but yours isn't sparse).

Separately, note that new Array() is better written [], and in fact you can put the entries directly in the square brackets rather than doing assignment after-the-fact. And of course, it's document (in all lower case), not Document with a capital D.

So putting those all together:

$(document).ready(function () {
    var images = [
        "#bannerImageOne",
        "#bannerImageTwo"
    ];
    var i;

    $('#homeCarousel img').hide();

    for (i = 0; i < images.length; ++i) {
        setTimeout(createFader(i, images), 5000);
    }
});

function createFader(index, array) {
    return function() {
        fadeInOut(index, array);
    };
}

function fadeInOut(i, images) {
    $(images[i]).fadeIn("slow").delay(2000).fadeOut();
    //alert(images[i]);
}

Also note that I moved the declaration of i to the top of the function, because that's where the JavaScript engine sees it anyway (details: Poor, misunderstood var).

Unless you have a really good reason why fadeInOut has to be a global function, I'd move it (and createFader) into the function you're passing to ready so we create no global symbols at all. That also lets them both use images directly, so we don't have to pass the array reference around (not that that's expensive, it's totally fine). That would look like this:

$(document).ready(function () {
    var images = [
        "#bannerImageOne",
        "#bannerImageTwo"
    ];
    var i;

    $('#homeCarousel img').hide();

    for (i = 0; i < images.length; ++i) {
        setTimeout(createFader(i, images), 5000);
    }

    function createFader(index) {
        return function() {
            fadeInOut(index);
        };
    }

    function fadeInOut(i) {
        $(images[i]).fadeIn("slow").delay(2000).fadeOut();
        //alert(images[i]);
    }
});

It works because by putting the functions inside the anonymous function you're passing into ready, we make them closures over the context of the call to that anonymous function. (See the link above about closures.) Even after the function returns (which it does almost immediately), the context is kept in memory and provides a handy container for our private data.

Recommendation for another way

Since all that the setTimeout call is doing is delaying the start of the fadeIn by 5 seconds, the code can be much simpler if you use delay (as you already have to put a pause between the fadeIn and fadeOut):

$(document).ready(function () {
    var images = [
        "#bannerImageOne",
        "#bannerImageTwo"
    ];

    $('#homeCarousel img').hide();

    $.each(images, function(index, id) {
        $(id).delay(5000).fadeIn("slow").delay(2000).fadeOut();
    });
});

Or keeping fadeInOut as a separate function:

$(document).ready(function () {
    var images = [
        "#bannerImageOne",
        "#bannerImageTwo"
    ];

    $('#homeCarousel img').hide();

    $.each(images, function(index, id) {
        fadeInOut(id, 5000);
    });

    function fadeInOut(id, delay) {
        $(id).delay(delay).fadeIn("slow").delay(2000).fadeOut();
    }
});

The delay takes the place of the setTimeout and all the images start to fadein after 5 seconds. (I also threw in an example of using $.each to lop through the array.) If you wanted them to fade in sequentially, you could multiple the delay by the index:

$.each(images, function(index, id) {
    $(id).delay(5000 * (index + 1)).fadeIn("slow").delay(2000).fadeOut();
});

or

$.each(images, function(index, id) {
    fadeInOut(id, 5000 * (index + 1));
});

...which would delay the first image by 5 seconds, the second by 10, etc.