javascript scope issue in recursive function

Question

In code below, I expect variable counter to incement by 1 on each repetition, but it remains zero.

var count = 0;
var marqueeText = new Array("Some text goes here",
                            "A bit more verbose text goes here.",
                            "Some more verbose <a href='xxx'>text</a> goes <a href='xxx'>here</a> with some <a href='xxx'>links</a>."
                            );

function fnShowMarquee(count){  
// console.log(count + "-" + marqueeText.length );
        if (count > marqueeText.length){
            count = 0;      
        }
        else{           
            count++;
        }
        $('#marquee_box span').html(marqueeText[count]);

            // do some stuff, then wait 4 seconds and call function again

        setTimeout ("fnShowMarquee(count)", 4000);
}


$(document).ready(function() {
    fnShowMarquee(0);
});

rats... gtg but hope this helps jsfiddle.net/62k5j

Joseph Marikle
– Joseph Marikle

2011-10-04 02:46:09 +00:00
Commented Oct 4, 2011 at 2:46 — Joseph Marikle
– Joseph Marikle, Commented Oct 4, 2011 at 2:46

James Khoury · Accepted Answer · 2011-10-04 03:36:56Z

3

The problem is you are using a global count then creating a new local in fnShowMarquee

Try this instead:

var count = 0;

//...

function fnShowMarquee(){  
    //...
    setTimeout (fnShowMarquee, 4000);
}

$(document).ready(function() {
    fnShowMarquee();
});

EDIT:- From RobG's comment:

$(document).ready(function() {
    var count = 0;
    //...

    function fnShowMarquee(){ ... }
    fnShowMarquee();
});

by putting it all in the ready function the count variable cannot be accessed by other code and it will not mess around in the window global scope.

edited Oct 4, 2011 at 3:36

answered Oct 4, 2011 at 2:48

James Khoury

22.6k4 gold badges36 silver badges66 bronze badges

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

2 Comments

RobG Over a year ago

Yes, but better to hold count in a closure so it isn't global and able to be messed around by other code.

James Khoury Over a year ago

@RobG Thats right! It will still be global the the closure scope and won't affect outside code. Or it can be moved into the ready function.

RobG · Accepted Answer · 2011-10-04 03:33:12Z

2

Further to James' answer, keep count and marqueeText in closures so they can't be messed around by other code:

var fnShowMarquee = (function() {
    var count = 0;
    var marqueeText = [
       "Some text goes here",
       "A bit more verbose text goes here.",
       "Some more verbose <a href='xxx'>text</a>" + 
       " goes <a href='xxx'>here</a> with some <a href='xxx'>links</a>."
    ];
    // ...                           

    return function() {  
        //...
        setTimeout (fnShowMarquee, 4000);
    }
}());

answered Oct 4, 2011 at 3:33

RobG

148k32 gold badges180 silver badges216 bronze badges

Comments

Blender · Accepted Answer · 2011-12-16 17:13:28Z

1

I'd try using count explicitly:

setTimeout (function() { fnShowMarquee(count); }, 4000);

edited Dec 16, 2011 at 17:13

answered Oct 4, 2011 at 2:44

Blender

300k55 gold badges462 silver badges511 bronze badges

8 Comments

Dave Newton Over a year ago

@MustaphaGeorge Why bother passing it in at all? It's already a global.

vol7ron Over a year ago

why not setTimeout(function(){fnShowMarquee(count);}, 4000) -- no need for creating a string

RobG Over a year ago

@vol7ron - because it can be just:setTimeout(fnShowMarquee, 4000) since count is global.

vol7ron Over a year ago

@RobG: that's if count is global. This is only a snippet, so it's possible he's running this elsewhere. I would advise not to pollute the global namespace.

RobG Over a year ago

Variable count can be anywhere on the scope chain, see other answers. It was global in the OP, you didn't change it, so that's where it is for now (though it should be removed as a formal parameter in the declaration of fnShowMarquee).

|

Poojan · Accepted Answer · 2011-10-04 03:59:38Z

Made a few changes to your code:

You are using a global count variable. You don't need to pass it in the parameters.
The if condition should be count == marqueeText.length - 1. In your previous code, marqueeText[count] was going off the bounds.
$('#marquee_box span') wasn't working for me. So I changed it to $('span#marquee_box')

Your code should be:

var count = 0;
var marqueeText = new Array("Some text goes here",
                            "A bit more verbose text goes here.",
                            "Some more verbose <a href='xxx'>text</a> goes <a href='xxx'>here</a> with some <a href='xxx'>links</a>."
                            );

function fnShowMarquee(){  
  console.log(count + "-" + marqueeText.length );
  if (count == marqueeText.length - 1){
      count = 0;      
  }
  else{           
      count++;
  }
  $('span#marquee_box').html(marqueeText[count]);

      // do some stuff, then wait 4 seconds and call function again

  setTimeout ("fnShowMarquee()", 4000);
}


$(document).ready(function() {
    fnShowMarquee();
});

Bruno Campos · Accepted Answer · 2012-02-07 21:19:05Z

0

You are increasing the parameter variable count. Not the global one. And the setTimeout like that uses the global one, because it runs your function on document scope.

Try to call your function as just

function fnShowMarquee()

or do it like

setTimeout (function() { fnShowMarquee(count); }, 4000);

edited Feb 7, 2012 at 21:19

answered Oct 4, 2011 at 2:47

Bruno Campos

2,1871 gold badge20 silver badges34 bronze badges

1 Comment

Chris Cooper Over a year ago

Don't pass strings to setTimeout, pass functions instead.

Collectives™ on Stack Overflow

javascript scope issue in recursive function

5 Answers 5

2 Comments

Comments

8 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

Comments

8 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related