2
\$\begingroup\$

I have two jQuery Ajax callbacks in succession, to fetch two sets of data from two different URLs, before combining them.

    $.ajax({
      type: "GET",
      url: numeratorUrl,
      error: function(request, status, error) {
        _this.showErrorMessage(request.status, error);
      },
      success: function(data) {
        parse(data,
             {columns: true },
             function(err, output){
                _this.globalOptions.data.numeratorData = output;
                $.ajax({
                  type: "GET",
                  url: denominatorUrl,
                  error: function(request, status, error) {
                    _this.showErrorMessage(request.status, error);
                  },
                  success: function(data) {
                    parse(data,
                         {columns: true },
                         function(err, newOutput){
                            _this.globalOptions.data.denominatorData = newOutput;
                            // combine the two datasets here
                         }
                    );
                  }
                });
             }
        );
      }
    });

It works fine, but I think it's unwieldy and hard to read. Is there a way to make it cleaner?

\$\endgroup\$
1
\$\begingroup\$

Yes you can.

In the first instance, you can extract the second call to a function

function secondCall () {
    $.ajax({
        type: "GET",
        url: denominatorUrl,
        error: function(request, status, error) {
            _this.showErrorMessage(request.status, error);
        },
        success: function(data) {
            parse(data, {columns: true }, function(err, newOutput) {
                _this.globalOptions.data.denominatorData = newOutput;
                // combine the two datasets here
             });
        }
    });

}

$.ajax({
      type: "GET",
      url: numeratorUrl,
      error: function(request, status, error) {
        _this.showErrorMessage(request.status, error);
      },
      success: function(data) {
        parse(data, {columns: true }, function(err, output) {
            _this.globalOptions.data.numeratorData = output;
            secondCall();
        });
      }
});

If you are using jQuery, you can also use the short methods instead of the low level interface. Since jquery 1.8, you can use done and fail (Mix ajax object with Deferred).

$.get(numeratorUrl)
     .done(function(data) {
        parse(data, {columns: true }, function(err, output) {
            _this.globalOptions.data.numeratorData = output;
            secondCall();
      })
     .fail(function(request, status, error) {
        _this.showErrorMessage(request.status, error);
      });

Also considerate if you can paralelize both requests, you can use Deferred object to analize when both calls ends.

   var call1 =  $.get(numeratorUrl);
   var call2 =  $.get(denominatorUrl);

   $.when(call1, call2).then(function (response1, response2) {
     //You have both responses at this point.

   });
\$\endgroup\$
1
\$\begingroup\$

If the ajax operation is simple, you can use the shorthand $.get instead. I usually use $.ajax when I alter default AJAX settings.

In addition, your code looks unwieldy because you are encountering what they call "callback pyramid". The way to avoid that is use Promises. And you're in luck, jQuery AJAX operations return promises.

Here's a non-pyramid version.

$.get(numeratorUrl).then(function(data) {
  // When get resolves
  var parseDeferred = $.Deferred();
  parse(data, { columns: true }, function(err, output) {
    if (err) parseDeferred.reject(err);
    else parseDeferred.resolve(output);
  });
  return parseDeferred.promise()
}).then(function(output) {
  // When parse resolves
  _this.globalOptions.data.numeratorData = output;
  return $.get(denominatorUrl)
}).then(function(data) {
  // When second get resolves
  var parseDeferred = $.Deferred();
  parse(data, { columns: true }, function(err, output) {
    if (err) parseDeferred.reject(err);
    else parseDeferred.resolve(output);
  });
  return parseDeferred.promise()
}).then(function(newOutput) {
  // When second parse resolves
  _this.globalOptions.data.denominatorData = newOutput;
}, function(error) {
  // Catch-all for all errors
  _this.showErrorMessage(null, error)
});

Would have been better if parse returned a promise or a value (if it was synchronous). If so, then the code would look like this:

$.get(numeratorUrl).then(function(data) {
  // when get resolves
  return parse(data, { columns: true })
}).then(function(output) {
  // when parse resolves
  _this.globalOptions.data.numeratorData = output;
  return $.get(denominatorUrl)
}).then(function(data) {
  // when second get resolves
  return parse(data, { columns: true });
}).then(function(newOutput) {
  // when second parse resolves
  _this.globalOptions.data.denominatorData = newOutput;
}, function(error) {
  // catch-all for all errors
  _this.showErrorMessage(null, error)
});

Here's the same code, explained:

$.get(numeratorUrl).then(function(data){
  // Listen for $.get response,

  // Unless parse returns a promise, we can use Deferreds to manually
  // create promises, resolve and reject. This is handy especially for
  // async operations that don't operate on promises.

  var parseDeferred = $.Deferred();
  parse(data, { columns: true }, function(err, output){
    if(err) parseDeferred.reject(err);
    else parseDeferred.resolve(output);
  });

  // We return the deferred as a promise, a read-only version of the deferred.
  // This means we lose the resolve and reject interface from the deferred.
  return parseDeferred.promise();

}).then(function(output){
  // Since we returned a promise from the last then, this callback now listens
  // to that promise instead of the $.get()'s promise.

  _this.globalOptions.data.numeratorData = output;

  // Now we do another AJAX call. We return it's value (which is a promise) which
  // means the next chained then listens to this promise instead of the one from
  // parse. Since $.get() returns a promise, we don't need to manually use Deferreds.
  return $.get(denominatorUrl);

}).then(function(data){

  // Use the same pattern as above
  var parseDeferred = $.Deferred();
  parse(data, { columns: true }, function(err, output){
    if(err) parseDeferred.reject(err);
    else parseDeferred.resolve(output);
  });
  return parseDeferred.promise();

}).then(function(newOutput){

  _this.globalOptions.data.denominatorData = newOutput;

}, function(error){
  // Now the nice thing about promises is that errors propagate the
  // same way resolved values propagate. Unless you return a new promise,
  // the succeeding thens will always resolve/reject with the value from
  // the latest resolved/reject promise.
  //
  // If the first $.get() failed, this will catch the error. If the
  // first succeeded, and the parse after it failed, this catches it.
  //
  // In jQuery, I believe their rejection resolves with 3 values,
  // the xhr object, text status and another status - which is not
  // now native promises work. So you might want to look after it.

  _this.showErrorMessage(null, error);
});
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.