Managing Ajax calls
up vote
1
down vote
favorite
I've gotten fed up of writing boilerplate code for managing Ajax calls, and this is the manager class I've come up with
The concept is that it standardizes the interface for RESTful calls, prevents duplicate calls from firing via a request key, also simplifies waiting for a call to complete.
function AjaxManager() {
this.processes = {};
this.ajax = (label, uri, settings, success, fail) => {
console.log({
ajax: {
label,
uri,
settings
}
});
this.abort(label);
this.processes[label] = $.ajax(
uri,
settings
).done((response) => {
console.log({
label,
success: "success",
response
});
if (success) {
success(response);
}
}).fail((response) => {
console.log({
label,
success: "fail",
response
});
if (fail) {
fail(response);
}
}).always(() => {
console.log("cleanup " + label);
this.processes[label] = null;
});
}
this.post = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "POST",
data: data
}, success, fail);
}
this.get = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "GET",
data: data
}, success, fail);
}
this.put = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "PUT",
data: JSON.stringify(data),
contentType: "application/json",
dataType: "json",
}, success, fail);
}
this.delete = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "DELETE",
data: data
}, success, fail);
}
this.wait = (label, callback) => {
if (this.processes[label]) {
this.processes[label].then(callback);
} else {
callback();
}
}
this.abort = (label) => {
if (this.processes[label]) {
this.processes[label].abort();
}
}
}
It looks to be working correctly but I wondered if you guys could give it a once over and check for anything I've missed that needs improving or fixing.
javascript jquery ajax
add a comment |
up vote
1
down vote
favorite
I've gotten fed up of writing boilerplate code for managing Ajax calls, and this is the manager class I've come up with
The concept is that it standardizes the interface for RESTful calls, prevents duplicate calls from firing via a request key, also simplifies waiting for a call to complete.
function AjaxManager() {
this.processes = {};
this.ajax = (label, uri, settings, success, fail) => {
console.log({
ajax: {
label,
uri,
settings
}
});
this.abort(label);
this.processes[label] = $.ajax(
uri,
settings
).done((response) => {
console.log({
label,
success: "success",
response
});
if (success) {
success(response);
}
}).fail((response) => {
console.log({
label,
success: "fail",
response
});
if (fail) {
fail(response);
}
}).always(() => {
console.log("cleanup " + label);
this.processes[label] = null;
});
}
this.post = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "POST",
data: data
}, success, fail);
}
this.get = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "GET",
data: data
}, success, fail);
}
this.put = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "PUT",
data: JSON.stringify(data),
contentType: "application/json",
dataType: "json",
}, success, fail);
}
this.delete = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "DELETE",
data: data
}, success, fail);
}
this.wait = (label, callback) => {
if (this.processes[label]) {
this.processes[label].then(callback);
} else {
callback();
}
}
this.abort = (label) => {
if (this.processes[label]) {
this.processes[label].abort();
}
}
}
It looks to be working correctly but I wondered if you guys could give it a once over and check for anything I've missed that needs improving or fixing.
javascript jquery ajax
add a comment |
up vote
1
down vote
favorite
up vote
1
down vote
favorite
I've gotten fed up of writing boilerplate code for managing Ajax calls, and this is the manager class I've come up with
The concept is that it standardizes the interface for RESTful calls, prevents duplicate calls from firing via a request key, also simplifies waiting for a call to complete.
function AjaxManager() {
this.processes = {};
this.ajax = (label, uri, settings, success, fail) => {
console.log({
ajax: {
label,
uri,
settings
}
});
this.abort(label);
this.processes[label] = $.ajax(
uri,
settings
).done((response) => {
console.log({
label,
success: "success",
response
});
if (success) {
success(response);
}
}).fail((response) => {
console.log({
label,
success: "fail",
response
});
if (fail) {
fail(response);
}
}).always(() => {
console.log("cleanup " + label);
this.processes[label] = null;
});
}
this.post = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "POST",
data: data
}, success, fail);
}
this.get = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "GET",
data: data
}, success, fail);
}
this.put = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "PUT",
data: JSON.stringify(data),
contentType: "application/json",
dataType: "json",
}, success, fail);
}
this.delete = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "DELETE",
data: data
}, success, fail);
}
this.wait = (label, callback) => {
if (this.processes[label]) {
this.processes[label].then(callback);
} else {
callback();
}
}
this.abort = (label) => {
if (this.processes[label]) {
this.processes[label].abort();
}
}
}
It looks to be working correctly but I wondered if you guys could give it a once over and check for anything I've missed that needs improving or fixing.
javascript jquery ajax
I've gotten fed up of writing boilerplate code for managing Ajax calls, and this is the manager class I've come up with
The concept is that it standardizes the interface for RESTful calls, prevents duplicate calls from firing via a request key, also simplifies waiting for a call to complete.
function AjaxManager() {
this.processes = {};
this.ajax = (label, uri, settings, success, fail) => {
console.log({
ajax: {
label,
uri,
settings
}
});
this.abort(label);
this.processes[label] = $.ajax(
uri,
settings
).done((response) => {
console.log({
label,
success: "success",
response
});
if (success) {
success(response);
}
}).fail((response) => {
console.log({
label,
success: "fail",
response
});
if (fail) {
fail(response);
}
}).always(() => {
console.log("cleanup " + label);
this.processes[label] = null;
});
}
this.post = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "POST",
data: data
}, success, fail);
}
this.get = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "GET",
data: data
}, success, fail);
}
this.put = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "PUT",
data: JSON.stringify(data),
contentType: "application/json",
dataType: "json",
}, success, fail);
}
this.delete = (label, uri, data, success, fail) => {
this.ajax(label, uri, {
method: "DELETE",
data: data
}, success, fail);
}
this.wait = (label, callback) => {
if (this.processes[label]) {
this.processes[label].then(callback);
} else {
callback();
}
}
this.abort = (label) => {
if (this.processes[label]) {
this.processes[label].abort();
}
}
}
It looks to be working correctly but I wondered if you guys could give it a once over and check for anything I've missed that needs improving or fixing.
javascript jquery ajax
javascript jquery ajax
edited Apr 28 at 0:50
Jamal♦
30.2k11115226
30.2k11115226
asked Apr 27 at 11:05
MikeT
17616
17616
add a comment |
add a comment |
1 Answer
1
active
oldest
votes
up vote
0
down vote
Instead of assigning all the methods to
this
inside one large function, just write a class:
class AjaxManager {
constructor() {
this.processes = {};
}
ajax() { ... }
post() { ... }
}
Why do you need the logging? I would just drop all the
console.log()
calls.
The required
label
parameter looks pretty inconvenient. I guess you need to use it like so:
manager.get("myLabel", "/some/url");
...
manager.abort("myLabel");
Why not simply return the promise itself:
const request = manager.get("/some/url");
...
request.abort();
This way, you don't need to come up with label names, especially when you never plan to abort. You would also remove the need for a "Manager"... which leads me to my next point.
The
AjaxManager
constructor takes no parameters. So I guess you create it once as a singleton object and use for all requests in your app. Better to avoid such global objects and create one per request.
Come to think of it... you probably don't need an object at all. You could replace this all with a single function:
ajax("POST", "/some/url");
Even the
success
andfail
params could be dropped. You could just use.done()
and.fail()
methods directly.
So in conclusion... this whole big AjaxManager
doesn't really seem to provide many benefits over the jQuery.ajax() API. Perhaps all you need is a simpler helper function to wrap it in.
thanks for your comments, using the old style of class design because of browser comparability, everything i can see says microsoft never added support to IE for the class keyword. the logging is there for testing purposes, once i am happy that the class is working correctly i'll strip it
– MikeT
Apr 27 at 14:22
the promises may be created on different event trigger ie a Leaflet Moveend which if the map has several lays will require one get per layer which i have no guarentee will have completed before the map is moved again, as such i can't be sure the promise will still be in scope which is why they are held in common manager distinguished by a unique key, this allows subsequent calls to abort an incomplete call that was spawned in the previous event trigger
– MikeT
Apr 27 at 14:32
you are quite right about me using it as a quazi singleton, though not globally but per scope so to use the above example of leaflet each map instance would have its own manager.
– MikeT
Apr 27 at 14:43
your comment about returning the promise so that i can use the default done and fail instead of passing in callback functions makes a lot of sense as it reduces the level of complexity nicely thank you
– MikeT
Apr 27 at 14:44
add a comment |
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
0
down vote
Instead of assigning all the methods to
this
inside one large function, just write a class:
class AjaxManager {
constructor() {
this.processes = {};
}
ajax() { ... }
post() { ... }
}
Why do you need the logging? I would just drop all the
console.log()
calls.
The required
label
parameter looks pretty inconvenient. I guess you need to use it like so:
manager.get("myLabel", "/some/url");
...
manager.abort("myLabel");
Why not simply return the promise itself:
const request = manager.get("/some/url");
...
request.abort();
This way, you don't need to come up with label names, especially when you never plan to abort. You would also remove the need for a "Manager"... which leads me to my next point.
The
AjaxManager
constructor takes no parameters. So I guess you create it once as a singleton object and use for all requests in your app. Better to avoid such global objects and create one per request.
Come to think of it... you probably don't need an object at all. You could replace this all with a single function:
ajax("POST", "/some/url");
Even the
success
andfail
params could be dropped. You could just use.done()
and.fail()
methods directly.
So in conclusion... this whole big AjaxManager
doesn't really seem to provide many benefits over the jQuery.ajax() API. Perhaps all you need is a simpler helper function to wrap it in.
thanks for your comments, using the old style of class design because of browser comparability, everything i can see says microsoft never added support to IE for the class keyword. the logging is there for testing purposes, once i am happy that the class is working correctly i'll strip it
– MikeT
Apr 27 at 14:22
the promises may be created on different event trigger ie a Leaflet Moveend which if the map has several lays will require one get per layer which i have no guarentee will have completed before the map is moved again, as such i can't be sure the promise will still be in scope which is why they are held in common manager distinguished by a unique key, this allows subsequent calls to abort an incomplete call that was spawned in the previous event trigger
– MikeT
Apr 27 at 14:32
you are quite right about me using it as a quazi singleton, though not globally but per scope so to use the above example of leaflet each map instance would have its own manager.
– MikeT
Apr 27 at 14:43
your comment about returning the promise so that i can use the default done and fail instead of passing in callback functions makes a lot of sense as it reduces the level of complexity nicely thank you
– MikeT
Apr 27 at 14:44
add a comment |
up vote
0
down vote
Instead of assigning all the methods to
this
inside one large function, just write a class:
class AjaxManager {
constructor() {
this.processes = {};
}
ajax() { ... }
post() { ... }
}
Why do you need the logging? I would just drop all the
console.log()
calls.
The required
label
parameter looks pretty inconvenient. I guess you need to use it like so:
manager.get("myLabel", "/some/url");
...
manager.abort("myLabel");
Why not simply return the promise itself:
const request = manager.get("/some/url");
...
request.abort();
This way, you don't need to come up with label names, especially when you never plan to abort. You would also remove the need for a "Manager"... which leads me to my next point.
The
AjaxManager
constructor takes no parameters. So I guess you create it once as a singleton object and use for all requests in your app. Better to avoid such global objects and create one per request.
Come to think of it... you probably don't need an object at all. You could replace this all with a single function:
ajax("POST", "/some/url");
Even the
success
andfail
params could be dropped. You could just use.done()
and.fail()
methods directly.
So in conclusion... this whole big AjaxManager
doesn't really seem to provide many benefits over the jQuery.ajax() API. Perhaps all you need is a simpler helper function to wrap it in.
thanks for your comments, using the old style of class design because of browser comparability, everything i can see says microsoft never added support to IE for the class keyword. the logging is there for testing purposes, once i am happy that the class is working correctly i'll strip it
– MikeT
Apr 27 at 14:22
the promises may be created on different event trigger ie a Leaflet Moveend which if the map has several lays will require one get per layer which i have no guarentee will have completed before the map is moved again, as such i can't be sure the promise will still be in scope which is why they are held in common manager distinguished by a unique key, this allows subsequent calls to abort an incomplete call that was spawned in the previous event trigger
– MikeT
Apr 27 at 14:32
you are quite right about me using it as a quazi singleton, though not globally but per scope so to use the above example of leaflet each map instance would have its own manager.
– MikeT
Apr 27 at 14:43
your comment about returning the promise so that i can use the default done and fail instead of passing in callback functions makes a lot of sense as it reduces the level of complexity nicely thank you
– MikeT
Apr 27 at 14:44
add a comment |
up vote
0
down vote
up vote
0
down vote
Instead of assigning all the methods to
this
inside one large function, just write a class:
class AjaxManager {
constructor() {
this.processes = {};
}
ajax() { ... }
post() { ... }
}
Why do you need the logging? I would just drop all the
console.log()
calls.
The required
label
parameter looks pretty inconvenient. I guess you need to use it like so:
manager.get("myLabel", "/some/url");
...
manager.abort("myLabel");
Why not simply return the promise itself:
const request = manager.get("/some/url");
...
request.abort();
This way, you don't need to come up with label names, especially when you never plan to abort. You would also remove the need for a "Manager"... which leads me to my next point.
The
AjaxManager
constructor takes no parameters. So I guess you create it once as a singleton object and use for all requests in your app. Better to avoid such global objects and create one per request.
Come to think of it... you probably don't need an object at all. You could replace this all with a single function:
ajax("POST", "/some/url");
Even the
success
andfail
params could be dropped. You could just use.done()
and.fail()
methods directly.
So in conclusion... this whole big AjaxManager
doesn't really seem to provide many benefits over the jQuery.ajax() API. Perhaps all you need is a simpler helper function to wrap it in.
Instead of assigning all the methods to
this
inside one large function, just write a class:
class AjaxManager {
constructor() {
this.processes = {};
}
ajax() { ... }
post() { ... }
}
Why do you need the logging? I would just drop all the
console.log()
calls.
The required
label
parameter looks pretty inconvenient. I guess you need to use it like so:
manager.get("myLabel", "/some/url");
...
manager.abort("myLabel");
Why not simply return the promise itself:
const request = manager.get("/some/url");
...
request.abort();
This way, you don't need to come up with label names, especially when you never plan to abort. You would also remove the need for a "Manager"... which leads me to my next point.
The
AjaxManager
constructor takes no parameters. So I guess you create it once as a singleton object and use for all requests in your app. Better to avoid such global objects and create one per request.
Come to think of it... you probably don't need an object at all. You could replace this all with a single function:
ajax("POST", "/some/url");
Even the
success
andfail
params could be dropped. You could just use.done()
and.fail()
methods directly.
So in conclusion... this whole big AjaxManager
doesn't really seem to provide many benefits over the jQuery.ajax() API. Perhaps all you need is a simpler helper function to wrap it in.
answered Apr 27 at 12:45
Rene Saarsoo
2,016714
2,016714
thanks for your comments, using the old style of class design because of browser comparability, everything i can see says microsoft never added support to IE for the class keyword. the logging is there for testing purposes, once i am happy that the class is working correctly i'll strip it
– MikeT
Apr 27 at 14:22
the promises may be created on different event trigger ie a Leaflet Moveend which if the map has several lays will require one get per layer which i have no guarentee will have completed before the map is moved again, as such i can't be sure the promise will still be in scope which is why they are held in common manager distinguished by a unique key, this allows subsequent calls to abort an incomplete call that was spawned in the previous event trigger
– MikeT
Apr 27 at 14:32
you are quite right about me using it as a quazi singleton, though not globally but per scope so to use the above example of leaflet each map instance would have its own manager.
– MikeT
Apr 27 at 14:43
your comment about returning the promise so that i can use the default done and fail instead of passing in callback functions makes a lot of sense as it reduces the level of complexity nicely thank you
– MikeT
Apr 27 at 14:44
add a comment |
thanks for your comments, using the old style of class design because of browser comparability, everything i can see says microsoft never added support to IE for the class keyword. the logging is there for testing purposes, once i am happy that the class is working correctly i'll strip it
– MikeT
Apr 27 at 14:22
the promises may be created on different event trigger ie a Leaflet Moveend which if the map has several lays will require one get per layer which i have no guarentee will have completed before the map is moved again, as such i can't be sure the promise will still be in scope which is why they are held in common manager distinguished by a unique key, this allows subsequent calls to abort an incomplete call that was spawned in the previous event trigger
– MikeT
Apr 27 at 14:32
you are quite right about me using it as a quazi singleton, though not globally but per scope so to use the above example of leaflet each map instance would have its own manager.
– MikeT
Apr 27 at 14:43
your comment about returning the promise so that i can use the default done and fail instead of passing in callback functions makes a lot of sense as it reduces the level of complexity nicely thank you
– MikeT
Apr 27 at 14:44
thanks for your comments, using the old style of class design because of browser comparability, everything i can see says microsoft never added support to IE for the class keyword. the logging is there for testing purposes, once i am happy that the class is working correctly i'll strip it
– MikeT
Apr 27 at 14:22
thanks for your comments, using the old style of class design because of browser comparability, everything i can see says microsoft never added support to IE for the class keyword. the logging is there for testing purposes, once i am happy that the class is working correctly i'll strip it
– MikeT
Apr 27 at 14:22
the promises may be created on different event trigger ie a Leaflet Moveend which if the map has several lays will require one get per layer which i have no guarentee will have completed before the map is moved again, as such i can't be sure the promise will still be in scope which is why they are held in common manager distinguished by a unique key, this allows subsequent calls to abort an incomplete call that was spawned in the previous event trigger
– MikeT
Apr 27 at 14:32
the promises may be created on different event trigger ie a Leaflet Moveend which if the map has several lays will require one get per layer which i have no guarentee will have completed before the map is moved again, as such i can't be sure the promise will still be in scope which is why they are held in common manager distinguished by a unique key, this allows subsequent calls to abort an incomplete call that was spawned in the previous event trigger
– MikeT
Apr 27 at 14:32
you are quite right about me using it as a quazi singleton, though not globally but per scope so to use the above example of leaflet each map instance would have its own manager.
– MikeT
Apr 27 at 14:43
you are quite right about me using it as a quazi singleton, though not globally but per scope so to use the above example of leaflet each map instance would have its own manager.
– MikeT
Apr 27 at 14:43
your comment about returning the promise so that i can use the default done and fail instead of passing in callback functions makes a lot of sense as it reduces the level of complexity nicely thank you
– MikeT
Apr 27 at 14:44
your comment about returning the promise so that i can use the default done and fail instead of passing in callback functions makes a lot of sense as it reduces the level of complexity nicely thank you
– MikeT
Apr 27 at 14:44
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f193068%2fmanaging-ajax-calls%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown