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?

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jul 14, 2015 at 10:33
\$\endgroup\$
0

2 Answers 2

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.
 });
answered Jul 15, 2015 at 13:19
\$\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);
});
answered Jul 15, 2015 at 13:14
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.