Are recursive AJAX calls a bad idea?

2019-04-23 17:58发布

问题:

I have a simple function to pull in an array of templates:

function getTemplates(names, done, templates, index) {
    if (!index) index = 0;
    if (!templates) templates = {};
    if (index === names.length) return done(templates);
    $.ajax({
        url: '/templates/' + names[index] + '.min.html',
        success: function (data, status, xhr) {
            templates[names[index++]] = data;
                return getTemplates(names, done, templates, index);
        }
    });
}

It seems logical to me to just go from one to the next until they are all retrieved, then callback to the calling function. But I'm curious if doing so has any bad side effects. I haven't seen any so far, but I don't want to go to production with this without first getting some insight on any potential problems.


UPDATE: With the help of Google and BenjaminGruenbaum I've devised a solution:

function getTemplatesAsync(names, done) {
    var calls = [];
    var templates = {};
    names.forEach(function (name, index) {
        calls.push(
            $.ajax({
                url: '/templates/' + names[index] + '.min.html',
                success: function (data, status, xhr) {
                    templates[names[index++]] = data;
                }
            })
        );
    });
    $.when.apply($, calls).done(function () {
        // using "templates" here feels fragile for some reason.  Am I wrong?
        return done(templates);
    });
}

I'm using templates here because I need to be able to refer to each template by name, but somehow it feels fragile or unreliable. Does this look like a safe thing to do?

回答1:

Your updated code is much better than your initial one, but it still has a few issues, the main one being mixing promises and callbacks instead of using language facilities (Return value) and not using mappings.

Some improvements can be:

  • returning the promise instead of a callback argument
  • using .map instead of forEach with push.
  • Using .then instead of a success callback for avoiding two handlers for the same thing and potentially unspecified behavior (does the when execute first? Does the success:?)

We can do something like:

function getTemplatesAsync(names) {
    return $.when.apply(null,names.map(function (name, index) {
        return $.get('/templates/' + names[index] + '.min.html');
    }).then(function(results){
         // map arguments to names
         return Array.prototype.reduce.call(arguments, function(obj,cur,idx){
               obj[names[idx]] = cur;
               return obj;
         },{});
    });
}

Which lets you do:

getTemplatesAsync(names).then(function(templates){
     // access templates here
});


回答2:

Yes. Making multiple AJAX calls this way is a bad idea, but possibly not for the reason that you think.

This is going to cause all of your calls to be executed sequentially rather than making the calls in parallel and waiting for them to finish that way.

You'd be far better off using promises to make all of your calls and then waiting for all of them to finish before continuing. It would look something like:

var promises = [], templates = [], i;
for(i = 0; i < names.length; i++) {
    promises.push($.get('/templates/' + names[i] + '.min.html'));
}

$.when.apply($, promises).done(function(responses) {
   for(i = 0; i < responses.length; i++) {
       templates.push(responses[i][0]);
   }
});


回答3:

Although it looks recursive (and I also use the term recursive Ajax for this) technically your function exits before it is called again, so is not actually recursive... perhaps we should call them "chained" Ajax calls as it is just chaining Async events together? :)

There is no problem doing it this way, if you do not mind queuing your Ajax request one at a time. I used it this way a few times to ensure bandwidth use was acceptable.

You need to be careful of edge cases, so that it handles server errors gracefully.

This is actually a good technique for handling mobile devices which will choke on a large number of requests going out at once.

I wrote have a plugin that loaded sections of a massive application form via Ajax and found devices like the iPad could not cope with more than a handful of simultaneous Ajax requests. I wound up using recursive/chained Ajax calls to solve the problem (and had the bonus of not choking our server too) :)