CODEWITHSUNDEEP
jquery
Teson
Teson

Reputation: 6736

why doesn't this code loop?

var fadeCounter = 0;

function fadeNext() {
  window.fadeCounter++;
  s = '#slide' + window.fadeCounter;
  $(s).fadeIn(1000,fadeNext);
  //forgive typos above, code works, what doesn't:
  if (window.fadeCounter == 8) window.fadeCounter = 0;
  //code will loop up to 8, then stop. why?
}

//initiate loop
fadeNext();

edit:

Demo added, at http://jsfiddle.net/AXRBh/2/ In demo, loop should reset at 20 but halts. Why?

Upvotes: 3

Views: 155

Answers (6)

Felix Kling
Felix Kling

Reputation: 816810

If fadeCounter is not declared in global scope, it will not work. Make sure you define the variable outside the document.ready handler (or remove window.).

The function does keep calling itself (only one time, see next paragraph) but you don't see the effect. After the first "loop" all the elements are visible so calling a second time fadeIn on them has no effect.

Because they are already visible, the fadeIn callback is fired immediately, making the function act in a weird way: http://jsfiddle.net/fkling/nZ7Mn/
In this case, the next fadeNext passed as callback is executed before the counter is reset:

  // s is already visible, so `fadeNext` is called immediately
  $(s).fadeIn(1000,fadeNext);
  // only after that the timer is reset (too late)
  if (window.fadeCounter == 8) window.fadeCounter = 0;

As there is not element with ID #slide9, calling fadeIn on it has no effect and the recursion stops.

As @Cybernate already noted, reversing the order of these two statements fixes the issue. However, you still won't see the effect as the elements are already visible. You just keep unnecessarily calling the function in a rapid way, which might even crash the browser.

So in order to make it working properly you should hide the elements again. Here is a bit cleaner version:

var max = 4;

function fadeNext(i) {
  i = i % max;
  $('body').append('<div>Current counter: ' + i + '</div>');
   $('#s' + i).fadeIn(1000,function() {
       $(this).fadeOut(1000, function(){fadeNext(i + 1)});
   });                                      
}

//initiate loop
fadeNext(0);

Instead of having the function accessing and changing a global variable (which can lead to some kind of "race condition" as you already experienced), pass the next index as parameter to the function.

DEMO

The big question is: What do you actually want to achieve?

Upvotes: 4

David Nguyen
David Nguyen

Reputation: 8508

What are you trying to do exactly? Is the function repeating indefinitely?

I did this demo:

http://jsfiddle.net/AXRBh/

$(document).ready(function() {
    var fadeCounter = 0;
    var time = setInterval(fadeNext, 2000);

    function fadeNext() {
       fadeCounter++;
       $('.slide').fadeOut(1000,function(){
          s = '#slide' + fadeCounter;
          $(s).fadeIn();
       });
       if (fadeCounter == 8)
          fadeCounter = 0;
    }
});

Upvotes: 1

Edgar Villegas Alvarado
Edgar Villegas Alvarado

Reputation: 18354

I think fadeCounter variable is out of scope (inside a function?). Do this:

//initiate loop
$(document).ready(function(){
   window.fadeCounter = 0;  //Explicitly reference it as  a global var
   fadeNext();
});

Your fadeNext() function declaration remains the same.

Upvotes: 0

Nick Brunt
Nick Brunt

Reputation: 10067

The simplest solution would be:

fadeLoop(0);

function fadeLoop(id) {
  if (id <= 8) { // add "&& id >= 0" if you're pedantic
    $("#slide" + id).fadeIn(1000, fadeLoop(id+1));
  }
}

Upvotes: 2

Josiah Ruddell
Josiah Ruddell

Reputation: 29831

Because you are using the incremented variable before checking to see if it is out of range.

The ninth time through the loop your code will try to $('#slide9').fadeIn

Fiddle working example

Upvotes: 0

Chandu
Chandu

Reputation: 82933

Move the counter reset right after the slide id.

var fadeCounter = 0;

function fadeNext() {
  window.fadeCounter++;
  s = '#slide' + window.fadeCounter;
  if (window.fadeCounter == 8) window.fadeCounter = 0;
  $(s).fadeIn(1000,fadeNext);
  //forgive typos above, code works, what doesn't:

  //code will loop up to 8, then stop. why?
}

//initiate loop
fadeNext();

Upvotes: 3

Related Questions