Jquery : Last image in array repeats twice, why?

Question

In my Array 'bg' the last item/image repeats twice when the function 'fadebg' gets to it, can anybody answer me why is this happening and how to fix it?

var bg = [];
bg[0] = "resources/images/bg/1.jpg";
bg[1] = "resources/images/bg/2.jpg";
bg[2] = "resources/images/bg/3.jpg";
bg[3] = "resources/images/bg/4.jpg";
bg[4] = "resources/images/bg/5.jpg";
bg[5] = "resources/images/bg/6.jpg";
bg[6] = "resources/images/bg/7.jpg";
bg[7] = "resources/images/bg/8.jpg";
bg[8] = "resources/images/bg/9.jpg";
bg[9] = "resources/images/bg/10.jpg";

var i = 0;

setInterval(fadebg, 10000);

function fadebg() {
    if (i < 10) {
        $('#bg').css({ opacity: 0 });
        setTimeout(function () {
            $('#bg').attr('src', bg[i]).css({ opacity: 1 })
        }, 300);
    };
    if (i == 10) {
        i = -1;
        $('#bg').css({ opacity: 0 });
        setTimeout(function () {
            $('#bg').attr('src', bg[i]).css({ opacity: 1 })
        }, 300);
        };
    i++;
};

And by the way, does anybody know's a shorter version of this function 'fadebg'?

Answer 1

Here's a simpler version:

(function() {
    var bgIndex = 0;
    setInterval(function() {
        var item = $('#bg').css({ opacity: 0 });
        setTimeout(function() {
            item.attr("src", bg[++bgIndex % bg.length]);
        }, 300);
    }, 10000);
})();

It uses the % trick to allow your index to cycle through the array without ever leaving the bounds of the array.

Also, this solves the original problem you had because the index variable is only incremented right when you go to use the index so there is no timing delay issue with when the variable is incremented and when it is used.

Summary of enhancements:

1. Use % bl.length trick to simplify code for staying within the bounds of the array and collapse your two branches of code to only one branch.
2. Increment array index only when we are just about to use it so there is no timing issue with when it's incremented.
3. Wrapped in IIFE so there are no globals. The global i is a very dangerous practice. One line of code that forgets the var in a for (i = 0;...) loop and now your global i gets trounced.
4. Saved value of $('#bg') so you're not regetting that a short time later.

Answer 2

It's because your functions inside fadebg close over the variable i, not the value it has as of when the function was created. They use the value it has when they run. This is the classic closure mistake. More: Closures are not complicated.

To use the value as of when the function is created, you have to use something other than i. I usually use a builder function:

function buildTheCallback(src) {
    return function () {
        $('#bg').attr('src', src).css({ opacity: 1 })
    };
}

...which you use like this:

setTimeout(buildTheCallback(bg[i]), 300);

Now, the function we're giving setTimeout (which is the function returned by the call to buildTheCallback closes over the src argument we passed into it, which doesn't get changed.

---

Re a shorter version, something along these lines (untested hey, what do you know, it works [I shortened the 10000ms to 1000ms for testing]):

(function() {
    var bg = [
        "resources/images/bg/1.jpg",
        "resources/images/bg/2.jpg",
        "resources/images/bg/3.jpg",
        "resources/images/bg/4.jpg",
        "resources/images/bg/5.jpg",
        "resources/images/bg/6.jpg",
        "resources/images/bg/7.jpg",
        "resources/images/bg/8.jpg",
        "resources/images/bg/9.jpg",
        "resources/images/bg/10.jpg"
        ];
    var bgIndex = -1;
    setInterval(fadeBg, 10000);

    function fadeBg() {
        bgIndex = (bgIndex + 1) % bg.length;
        $('#bg').css({ opacity: 0 });
        setTimeout(showBg, 300);
    }
    function showBg() {
        $('#bg').attr('src', bg[bgIndex]).css({ opacity: 1 })
    }
})();

(I'm assuming that the real paths to the images aren't identical except for the number; if they are, you can make this markedly shorter.)

But if it were me, I'd fade rather than just suddenly going to opacity: 0 / opacity: 1: Live Example

(function() {
    var bg = [
        "resources/images/bg/1.jpg",
        "resources/images/bg/2.jpg",
        "resources/images/bg/3.jpg",
        "resources/images/bg/4.jpg",
        "resources/images/bg/5.jpg",
        "resources/images/bg/6.jpg",
        "resources/images/bg/7.jpg",
        "resources/images/bg/8.jpg",
        "resources/images/bg/9.jpg",
        "resources/images/bg/10.jpg"
        ];
  var bgIndex = -1;
  setInterval(fadeBg, 10000);

  function fadeBg() {
    bgIndex = (bgIndex + 1) % bg.length;
    $('#bg').fadeTo("300", 0, showBg);
  }
  function showBg() {
    $('#bg').attr('src', bg[bgIndex]).fadeTo("300", 1);
  }
})();

Answer 3

try:

function fadebg() {
    if( i == 10 ){
      i = -1;
    }
    $('#bg').css({ opacity: 0 });
    setTimeout(function () {
      $('#bg').attr('src', bg[i]).css({ opacity: 1 })
    }, 300);
    i++;
};

Answer 4

You might be able to do something like this to make your function smaller:

bg.each(function(i){
    $('#bg').css({opacity:0});
    setTimeout(function () {
        $('#bg').attr('src', bg[i]).css({ opacity: 1 })
    }, 300);
});

I haven't tested that yet, but it might be something to look into.