HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Reusable Ajax object

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
reusableajaxobject

Problem

I've been working on a new project and I've found myself needing to make multiple Ajax calls, often on the same page and at the same time. I decided rather than repeating code that I would try and make a reusable object that to handle these calls.

I'm very new to Object Orientated JavaScript so I'm looking for some feedback and advice on my code; am I doing it right? is there something I'm not doing? should I be doing things differently etc?

Here's my Ajax constructor:

/**
 *
 * AjaxRequest Constructor
 *
 * @param url
 * @param success_callback
 * @param limit
 * @param type
 * @constructor
 */

function AjaxRequest (url, success_callback, limit, type) {
    this.url = url;
    this.success_callback = success_callback;
    this.limit =  limit || 3;
    this.type = type || "POST";

    //init ajax
    this.init();
}

/**
 * initialise
 * ajax
 */

AjaxRequest.prototype.init = function () {

    var _self = this;

    $.ajax({
        dataType: "json",
        url: this.url,
        type: this.type,
        data: {
            member_id: 1,
            limit: this.limit
        },
        beforeSend : function (){
            console.log('loading');
            //spinner.spin(target);
        },
        success: function (response) {
            if (response.success === 1) {
                _self.success_callback(response);
            }
        },
        error: function (a, b, c) {

        }
    });
}

/**
 * change number of
 * displayed outputs
 * @param limit
 */

AjaxRequest.prototype.set_limit = function (limit) {
    this.limit = limit;
    this.init();
}


and here is how I'm calling it

```
/**
* instantiate getAchievements
* @type {AjaxRequest}
*/

getAchievements = new AjaxRequest("/ajax-member/get-achievements", function (response) {

var i,
container = document.getElementsByClassName('achievements-container')[0],
achievements_data = '';

for (i in response.achievements) {
achievements_data += ("" + response

Solution

From a quick once over:

  • JsHint.com has very little feedback, good!



  • Consider using use strict to activate strict mode



  • Avoid console.log



  • Both beforeSend , error and complete should be customizable by the caller



  • success() returns ( PlainObject data, String textStatus, jqXHR jqXHR ) you should pass all 3 to success_callback



  • JavaScript is lowerCamelCase, success_callback -> successCallback



  • member_id: 1,



  • I wonder if AjaxRequest should know about "/ajax-member/, from a DRY perspective



  • for (i in response.achievements) {, response.achievements seems to be an array, please use a regular for loop to iterate over arrays, it will prevent hard to find bugs at a later stage



  • Find a good templating library, you need it, consider Handlebars



  • I think your Ajax class could be expanded, getAchievements and getComments are clearly a result of copy pasting code

Context

StackExchange Code Review Q#51289, answer score: 7

Revisions (0)

No revisions yet.