I got this javascript recursive function:

function doStuff(graphName) {
    var groupArray = new Array();
    groupArray[0] = "hour";
    groupArray[1] = "day";
    groupArray[2] = "month";
    for(var i = 0; i < groupArray.length; i++) {
        $.get("getchartdata", {"graphName" : graphName, "subgroup" : groupArray[i]})
              .done(function(jsonData){
                  var data = eval(jsonData);
                  drawChart(data, data[0][0], data[0][1]);
              });

    }
    setTimeout(doStuff, 10000);
}

Now the problem is that it works great the first time, but after 10 seconds when tries again, it shows an error:

TypeError: data[0] is undefined in drawChart(data, data[0][0], data[0][1]);

Why could this be happening?

If I add the parameter in setTimeout(doStuff(graphName), 10000);

The browser crashes.

Thanks.

I got this javascript recursive function:

function doStuff(graphName) {
    var groupArray = new Array();
    groupArray[0] = "hour";
    groupArray[1] = "day";
    groupArray[2] = "month";
    for(var i = 0; i < groupArray.length; i++) {
        $.get("getchartdata", {"graphName" : graphName, "subgroup" : groupArray[i]})
              .done(function(jsonData){
                  var data = eval(jsonData);
                  drawChart(data, data[0][0], data[0][1]);
              });

    }
    setTimeout(doStuff, 10000);
}

Now the problem is that it works great the first time, but after 10 seconds when tries again, it shows an error:

TypeError: data[0] is undefined in drawChart(data, data[0][0], data[0][1]);

Why could this be happening?

If I add the parameter in setTimeout(doStuff(graphName), 10000);

The browser crashes.

Thanks.

• javascript
• function
• recursion

• 1 Do never ever use eval there! (Not for JSON parsing, and not when it is already parsed) – Bergi Commented Apr 29, 2013 at 16:13
• it's not a recursive function. – Zo72 Commented Apr 29, 2013 at 16:16
• Remember that $.get is asynchronous so calling doSomething before all calls plete can cause problems if the calls take longer than 10 seconds to plete. You may want to use $.ajax and set async = false. – ron tornambe Commented Apr 29, 2013 at 16:21
• @rontornambe no, never ever use async: false ! – Alnitak Commented Apr 29, 2013 at 16:22
• Although I do not believe in hard and fast rules, in this case I do agree async would be a bad idea. A better solution is to include a counter should and not call doSOmething until groupArray.length is reached. – ron tornambe Commented Apr 29, 2013 at 16:31

3 Answers 3

I think what you want is this:

setTimeout(function() { 
    doStuff(graphName); 
}, 10000);

It is also worth noting that if your AJAX call takes more than 10 seconds to plete you may start to see some 'glitches'. Perhaps consider moving the timeout to inside the .done callback (this would mean it run again 10 seconds after the ajax has pleted). However, this is just a suggestion, if this doesn't suit your needs then you can keep it as it is. Also, this may not be suitable as you are calling the ajax in a for loop, and you may end up with more timeouts than you want, if you don't implement it correctly

If you want to pass on the graphname parameter, you'll need to explicitly do so! Seems like you want

function doStuff(graphName) {
    var groupArray = ["hour", "day", "month"];
    for(var i = 0; i < groupArray.length; i++) {
        $.get("getchartdata", {"graphName" : graphName, "subgroup" : groupArray[i]})
              .done(function(data){
                  drawChart(data, data[0][0], data[0][1]);
              });
    }
    setTimeout(function() {
        doStuff(graphName); // again
    }, 10000);
}

Another possibility would be to bind the argument for the next doStuff invocation:

setTimeout(doStuff.bind(this, graphName), 10000);

It's usual in this case to do the hard work in an inner function, with the originally supplied parameter available via the closure:

function doStuff(graphName) {
    (function loop() {
         // draw the graph using "graphName" from the outer scope
         ...
         setTimeout(loop, 10000);
    })();  // invoke immediately to start the process
}

using the closure avoids the repetition of passing the parameters over and over, and the additional function wrapper around that call, since you can just pass the reference to the inner function.

This also works well with AJAX - just put the setTimeout call inside the .done handler. As you're making three AJAX calls, try this in the inner function which will wait for all three AJAX calls to plete before starting the timer:

var def = [];

for (var i = 0; i < groupArray.length; i++) {
    def[i] = $.get("getchartdata", {"graphName" : graphName, "subgroup" : groupArray[i]})
          .done(function(data) {
              drawChart(data, data[0][0], data[0][1]);
          });
}

// wait for all three deferred objects to be resolved
$.when.apply($, def).done(function() { setTimeout(loop, 10000) });