3
\$\begingroup\$

I have created an central AJAX function (using JQuery) which will be responsible for handling all the AJAX calls in my project.

var ajaxCall = function(_url, _type, _data, _successCallback, _errorCallback, isAsync, _completeCallback){          

    // Handling _url
    if(!_url.trim()) throw new Error('Sucess callback is not defined');

    // Handling _type
    if(!_type.trim()) throw new Error('TYPE is not defined');

    // Handling _successCallback
    if(!_successCallback || typeof _successCallback !== "function") throw new Error('Success callback is not defined');

    // Handling _errorCallback
    if(!_errorCallback || typeof _errorCallback !== "function") _errorCallback = function() {};

    // Handling _completeCallback
    if(!_completeCallback || typeof _completeCallback !== "function") _completeCallback = function() {};

    // AJAX configuration object
    var ajaxObj = {
        url : _url.trim(),
        contentType : "application/json",
        type : _type.trim(),
        data : _data || {},
        async: !!isAsync,
        success : function(data) {
            try {
                _successCallback(data);
            } catch (exception) {
                global.logger.i(TAG, 'Exception! ' + exception);
            }
        },
        error : _errorCallback,
        complete: _completeCallback
    };

    // Capturing AJAX call
    return $.ajax(ajaxObj);

};

Let me know if this is a valid way to centralize the AJAX calls.

\$\endgroup\$
  • \$\begingroup\$ really? async: isAsync !== false ? true : false why? Other than your log call I see no real benefit in using a delegation function like that, it only masks the properties as function parameters and makes it less readable IMHO. \$\endgroup\$ – xander May 2 '17 at 13:37
  • \$\begingroup\$ I do agree with your point on async. I have changed it to async: !!isAsync. Only purpose of using functions like this is to have a central declaration of AJAX so that there is no need of repeating $.ajax again and again. Instead it will be a simple function call i.e. ajaxCall(...). \$\endgroup\$ – Ankur Gupta May 2 '17 at 14:19
  • \$\begingroup\$ Why not just use async: isAsync? And I understand what you mean, but it depends how you actually use the $.ajax function in your code, usually you don't need all the properties every time so the actual $.ajax call is way shorter than your complete call there!? And the problem with functions with a lot of parameters is readability, unless your IDE shows the parameter names (some do that). I mean if you call a function like fun("foo", 1, true, 3, "no") who knows what those arguments are, that's why a JS object is better to read. :) \$\endgroup\$ – xander May 2 '17 at 14:24
3
\$\begingroup\$

Central ajax function in jquery

No. Just, no. It's tempting to handle how all AJAX operations are called in your app, but this is usually just for a handful of use cases. I've seen code time and again do this only to actually call $.ajax directly elsewhere because the "global" one just doesn't serve its needs.

var ajaxCall = function(_url, _type, _data, _successCallback, _errorCallback, isAsync, _completeCallback){ 

jQuery AJAX methods return a promise-like object. Use the standard promise interface to assign callbacks instead of using the old callback method. That means, use the then method. Using this also makes your code easily portable to standard APIs like fetch.

Additionally, there's no sense adding _ before variable names. It only adds noise. If you mean to avoid collisions, just use different variable names. Don't make it more complicated than it already is.

// Handling _url
if(!_url.trim()) throw new Error('Sucess callback is not defined');

// Handling _type
if(!_type.trim()) throw new Error('TYPE is not defined');

// Handling _successCallback
if(!_successCallback || typeof _successCallback !== "function") throw new Error('Success callback is not defined');

// Handling _errorCallback
if(!_errorCallback || typeof _errorCallback !== "function") _errorCallback = function() {};

// Handling _completeCallback
if(!_completeCallback || typeof _completeCallback !== "function") _completeCallback = function() {};

I also wouldn't do these param checks and preprocessing if I were you. A function should assume the arguments are correct. That way, it fails on bad input. It's also the duty of the consuming code to use the function correctly. Don't make it just catch the other code's fault. Let the function fail on bad input, let the browser throw an error or do a bad request. A visible error on the network or console is a bazillion times better than some function swallowing errors.

async: isAsync !== false ? true : false,

Synchronous XHR should never be an option. It freezes the UI and that's bad for UX. Learn how to write and deal with asynchronous code. Don't work around it.

try {
  _successCallback(data);
} catch (exception) {
  global.logger.i(TAG, 'Exception! ' + exception);
}

Lastly, your function should not be concerned about the callback. It's none of its concern. Let the callback handle its own errors. Also, globally swallowing errors is a bad idea. Errors are harder to trace. ANd if your logger is disabled, you will never know the callback failed.

In the end, the call you aim to globalize is nothing more than just the following. $.get is an explicit shorthand for $.ajax({ method: 'get' }) ($.ajax defaults to GET but you'd have to know jQuery by heart to know that). type can be omitted if your server returns the correct content type response headers for JSON so jQuery knows what to do with it.

$.get({
  contentType: 'application/json',
  data: { ... },
  url: '...'
}).then(function(res){
  // AJAX succeeded
}, function(err){
  // AJAX failed
});
\$\endgroup\$
  • \$\begingroup\$ +1 Couldn't agree more that this proposed function is a wrapper that adds no value beyond the functions currently exposed by jQuery. It adds complexity and reduces compatibility with other code that works with jQuery as intended. If you are going to add complexity and reduce compatibility your better have a very good value proposition for doing so. \$\endgroup\$ – Mike Brant May 2 '17 at 17:48

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.