[update: keeping this around for posterity, but we ultimately went to a pub/sub pattern, using Amplify.js. Amplify isn't strictly necessary to implement pub/sub, but it has some nice features and syntax]
I have a REST API, which naturally you can just use HTTP calls (mainly Ajax) in order to send and receive data. I am building a layer of abstraction on top of the raw REST API for JavaScript Developers. I already use jQuery in the project, so I am using $.ajax()
. I feel like I have arrived at a good pattern, but I'm not absolutely certain.
var ta = ta || {};
$.ajaxSetup({
cache: false
});
ta.api = {
heartbeat: function(successCallback, errorCallback) {
var callType = "heartbeat";
// if no custom handlers are provided, wire up the default callbacks or blank callback methods
successCallback = ta.utils.setDefaultIfNeeded(successCallback, ta.callbacks[callType].success);
errorCallback = ta.utils.setDefaultIfNeeded(errorCallback, ta.callbacks[callType].error);
var url = '/rs/heartbeat';
$.ajax({
url: url,
success: function(data) {
successCallback(data);
},
error: function(xhr) {
errorCallback(xhr);
}
});
},
version: function(successCallback, errorCallback) {
var callType = "version";
// if no custom handlers are provided, wire up the default callbacks or blank callback methods
successCallback = ta.utils.setDefaultIfNeeded(successCallback, ta.callbacks[callType].success);
errorCallback = ta.utils.setDefaultIfNeeded(errorCallback, ta.callbacks[callType].error);
var url = '/rs/version';
$.ajax({
url: url,
success: function(data) {
successCallback(data);
},
error: function(xhr) {
errorCallback(xhr);
}
});
}
}
ta.utils = {
setDefaultIfNeeded: function (providedHandler, defaultHandler) {
var checkedHandler = providedHandler;
if (typeof (providedHandler !== "undefined")) {
if (typeof (providedHandler) !== "function") {
if (typeof (defaultHandler) === "function") {
checkedHandler = defaultHandler;
} else {
checkedHandler = function () { /* intentionally empty */ };
}
}
} else {
checkedHandler = function () { /* intentionally empty */ };
}
return checkedHandler;
}
};
So, the API can accept success and error handlers on a per-call basis. The setDefaultIfNeeded
will do a few pieces of logic, but one of those pieces is an optional phase of using handlers from a callback object. So the callback object is optional, but it's a handy place to store and organize callbacks:
ta.callbacks = {
heartbeat: {
success: function(data) { console.log(data) },
error: function(xhr) { console.log(xhr.statusText) }
},
version: {
success: function(data) { console.log(data) },
error: function(xhr) { console.log(xhr.statusText) }
}
}
Another addition I plan to make but haven't tested yet is to have the functions return the jqXhr. It's already being made, so why not return it? But I haven't tested that yet so I didn't want to provide sample code that might be broken.
So, there are now a few options for using the API, depending on your needs:
- Make calls to ta.api methods and supply success and error handlers as needed. I.e. you would define them in the methods that ultimately call the REST API.
- Make void calls to ta.api, but the success and error handlers are supplied in the ta.callbacks object.
- You can still continue to make calls to the REST service with whatever other arbitrary methods you choose.
- *planned - define a variable against ta.api which becomes a jqXhr. You then have full access to the XHR and can do with it as you please.
I have a suspicion that some of you will recommend deferred/promises API, and I am not against it. But if I'm being honest, I can't visualize a strong way to encapsulate the various moving parts and have them extremely readable for other devs.
-
\$\begingroup\$ Please consider using promises. If you find them less readable, you need to spend more time with them, as many of us did before we saw the light. Promises are great for encapsulating the various moving parts because they have a simple interface which simply propagates values and errors. They can be composed. Taking a callback is fine, but return a promise. \$\endgroup\$Aluan Haddad– Aluan Haddad2016年06月25日 11:18:00 +00:00Commented Jun 25, 2016 at 11:18
-
\$\begingroup\$ Thanks for the comment, @AluanHaddad. In the end, we adopted a pub/sub pattern. Success and Error events simply publish the success (with its data) or its error (with the xhr). \$\endgroup\$Greg Pettit– Greg Pettit2016年06月27日 17:51:15 +00:00Commented Jun 27, 2016 at 17:51
1 Answer 1
Looks to me like there's a lot of duplication here. I.e. boilerplate stuff you'd probably end up copy/pasting (and having to deal with the consequences of that later).
I'd just pare it way down for now: All that really changes is the URL. A really simple version might be:
ta.api = (function ($) {
// private helper function
function get(url, success, error) {
return $.ajax({
url: url,
success: success || ta.callbacks[url].success;
error: error || ta.callbacks[url].error;
});
}
return {
heartbeat: function (success, error) {
return get('/rs/heartbeat', success, error);
},
version: function (success, error) {
return get('/rs/version', success, error);
}
};
}(jQuery));
There are two assumptions/changes here:
- You'd use the url as the property name for default callbacks to avoid having to maintain a separate
callType
variable, and - It assumes that you either pass a function or a false'y value (e.g.
null
or just nothing) for the callbacks. I.e. don't pass something truthy that's not a function.
Similarly, it assumes that ta.callbacks[url].*
is either undefined
or a proper function.
However, there's a potential bug if ta.callbacks[url]
itself is undefined
. So a safer option would be:
function get(url, success, error) {
return $.ajax({
url: url,
success: success || (ta.callbacks[url] ? ta.callbacks[url].success : null);
error: error || (ta.callbacks[url] ? ta.callbacks[url].error : null);
});
}
If you want to keep your setDefaultIfNeeded
, there are few issues with it. For one it's name is not terribly decriptive: Set a default what? A string? A number? It's very specifically a type-checking function that only looks for functions, but given the name, I might think I could use it for anything. And despite its name, it doesn't actually set anything. It just returns, so it should rather be called get...
.
Secondly, it can be shortened considerably:
ta.utils = {
getValidCallback: function (providedHandler, defaultHandler) {
if(jQuery.isFunction(providedHandler)) return providedHandler;
if(jQuery.isFunction(defaultHandler)) return defaultHandler;
return function () {};
}
};
I honestly don't have a good idea for a name (getValidCallback
is ok, but not great), because I'd just stick to the first option of using ||
and simply not have this function.
As for using promises/deferreds instead, the above code allows for that automatically. The result of $.ajax
is itself a promise object, and since this code returns it (your function didn't return anything), you can use it both ways:
ta.api.heartbeat(foo, bar);
// or
ta.api.heartbeat().then(foo, bar);
-
\$\begingroup\$ Thank you so much for taking the time, Flambino. I'll need some time to test this all out, but it seems every effective; particularly the ability to use the $.ajax promise object but also for the streamlined code. \$\endgroup\$Greg Pettit– Greg Pettit2015年12月07日 18:21:41 +00:00Commented Dec 7, 2015 at 18:21