Michael Tallino
Michael Tallino

Reputation: 821

Why is multiple ajax query loop running success twice?

So everything works, the problem is if conflicts are found it calls showInstConflicts(allconflicts, joined); twice... Debugging shows it evaluates true twice but I can't figure out how it possible could. When there are no conflicts, it doesn't call the generatePDF() function twice.

The form I'm using checks the proposed schedule (the form we're on) against an instructor's existing schedule to see if there are conflicts. Sometimes the instructor can have multiple meeting times for one class like:

  1. MWF, 1730-1930, Jan 12 - May 5, BLDG ROOM
  2. TR, 1200-0100, Jan 12 - May 5, BLDG ROOM2

In that case, the ajax check needs to run twice to make sure that BOTH meetings times have no conflicts.

The loop is counting the number of times with nodata found, checking to see if it's checked enough times (joined.length which is an array of meeting times to check) and if there was at least one time when data (aka conflicts) was received. If it finds at least one success call it calls the display function showInstConflicts() which displays the conflicts, after all the checks have finished (i==times).

If nodata counts up

I'm using a personal jQuery plugin .JSONTable() which simply makes an $.ajax call and formats it into a table based on defined settings. The nodata: is called when the ajax return data.length < 1.

So here's the loop:

var times = joined.length;
var nodata = 0;
var allconflicts = $('<div></div>');

for (i = 0; i < times; i++) {
    var conflicts = $('<div></div>').appendTo(allconflicts);
    //use $.JSONTable plugin (see plugins.js) to check instructor conflicts
    conflicts.JSONTable({
        url: '/ajax/ajaxCSFInstCheck.php',
        options: joined[i],
        noWraps: ['allrows'],
        columns: ['Name', 'Primary', 'Responsible', 'CRN', 'Class', 'Dates', 'Days', 'Times', 'Location', 'XST', 'Web', 'All Meetings'],
        responsive: true,
        success: function () {
            //if conflicts found, show on the final ajax return
            //*** this is evaluating true twice per run ***
            if (i == times && nodata < times) {
                showInstConflicts(allconflicts, joined);
            }
        },
        nodata: function () {
            //if no conflicts found, make the PDF
            nodata++;
            if (i == times && nodata == times) {
                generatePDF();
            }
        }
    });

}

The code for $.fn.JSONTable is on my website http://curric.uaa.alaska.edu/js/plugins.js, second half of the file.

Upvotes: 0

Views: 856

Answers (1)

jfriend00
jfriend00

Reputation: 707876

So, it looks like JSONTable is an Ajax call. That means it's asynchronous. As such, your for loop will immediately launch all the async calls and the index i will be at the value at the end of the loop when the first one finishes. Plus your Ajax calls may finish in any random order.

If you want to see the correct value of i that corresponds to that specific Ajax call, then you'd have to put i into a closure so it is preserved uniquely for each Ajax call.

If you want your Ajax calls to be executed in strict sequential order, then you will have to restructure how you run them (you can't use a straight for loop).

Here's how you would preserve the value of i for each separate ajax call:

var times = joined.length;
var nodata = 0;
var allconflicts = $('<div></div>');

for (var i = 0; i < times; i++) {
    // create closure to capture the value of i
    (function(i) {
        var conflicts = $('<div></div>').appendTo(allconflicts);
        //use $.JSONTable plugin (see plugins.js) to check instructor conflicts
        conflicts.JSONTable({
            url: '/ajax/ajaxCSFInstCheck.php',
            options: joined[i],
            noWraps: ['allrows'],
            columns: ['Name', 'Primary', 'Responsible', 'CRN', 'Class', 'Dates', 'Days', 'Times', 'Location', 'XST', 'Web', 'All Meetings'],
            responsive: true,
            success: function () {
                //if conflicts found, show on the final ajax return
                //*** this is evaluating true twice per run ***
                if (i == times && nodata < times) {
                    showInstConflicts(allconflicts, joined);
                }
            },
            nodata: function () {
                //if no conflicts found, make the PDF
                nodata++;
                if (i == times && nodata == times) {
                    generatePDF();
                }
            }
        });
    })(i);
}

I'm having a hard time following how exactly you're trying to process all this data. Generally, if you're retrieving N pieces of async data and some operations you want to do on this data needs to look at more than one async result, then the simplest approach is to collect all the data (usually into an array) and then process all the results at once at the end when you have all the data. Then, any inter-result dependencies can be looked at once.

When doing this with multiple ajax calls, it is generally simplest to use promises and let the promises infrastructure collect all the results and deliver it to your callback in an array when all the ajax calls are done (probably using Promise.all()).

Upvotes: 2

Related Questions