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.










share|improve this question




























    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.










    share|improve this question


























      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.










      share|improve this question















      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






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited Apr 28 at 0:50









      Jamal

      30.2k11115226




      30.2k11115226










      asked Apr 27 at 11:05









      MikeT

      17616




      17616






















          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 and fail 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.






          share|improve this answer





















          • 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











          Your Answer





          StackExchange.ifUsing("editor", function () {
          return StackExchange.using("mathjaxEditing", function () {
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          });
          });
          }, "mathjax-editing");

          StackExchange.ifUsing("editor", function () {
          StackExchange.using("externalEditor", function () {
          StackExchange.using("snippets", function () {
          StackExchange.snippets.init();
          });
          });
          }, "code-snippets");

          StackExchange.ready(function() {
          var channelOptions = {
          tags: "".split(" "),
          id: "196"
          };
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function() {
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled) {
          StackExchange.using("snippets", function() {
          createEditor();
          });
          }
          else {
          createEditor();
          }
          });

          function createEditor() {
          StackExchange.prepareEditor({
          heartbeatType: 'answer',
          convertImagesToLinks: false,
          noModals: true,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          imageUploader: {
          brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
          contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
          allowUrls: true
          },
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          });


          }
          });














          draft saved

          draft discarded


















          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

























          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 and fail 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.






          share|improve this answer





















          • 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















          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 and fail 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.






          share|improve this answer





















          • 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













          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 and fail 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.






          share|improve this answer














          • 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 and fail 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.







          share|improve this answer












          share|improve this answer



          share|improve this answer










          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


















          • 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


















          draft saved

          draft discarded




















































          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.




          draft saved


          draft discarded














          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





















































          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







          Popular posts from this blog

          Сан-Квентин

          Алькесар

          Josef Freinademetz