Reusable Ajax object

Question

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 += ("<div class='achievement'><div class='copy'><h2>" + response.achievements[i].t_achievement_name + "</h2><span>" + response.achievements[i].t_achievement_description + "</span></div></div>");
    }

    container.innerHTML = achievements_data;
});

/**
 * instantiate getComments
 * @type {AjaxRequest}
 */

getComments = new AjaxRequest("/ajax-member/get-comments", function (response) {

    var i,
        container = document.getElementsByClassName('comments')[0],
        comments_data = '';

    for (i in response.comments) {
        comments_data += ("<div class='comment'><div class='copy'><h2>" + response.comments[i].t_fname + " " + response.comments[i].t_lname + "</h2><span>" + response.comments[i].t_comment_content + "</span></div></div>");
    }

    container.innerHTML = comments_data;
});

It would be good to hear some feedback on this. I'm always looking to improve and find the best ways of doing things so if you spot something I should be doing differently then please shout.

Answer 1

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, <- Maybe this is sample code, this magic constant deserves a comment
• 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

Answer 2

The name of your class AjaxRequest is a generic name, yet it is used for a specific kind of request being that the limit and member_id parameters are hard coded. I think what you want is a class that encapsulates specific AJAX calls. For this, the Repository Pattern would work well:

function AchievementsRepository(baseUrl, memberId) {
    this.baseUrl = baseUrl;
    this.memberId = memberId;
}

AchievementsRepository.prototype = {

    baseUrl: null,

    memberId: null,

    constructor: AchievementsRepository,

    add: function(data) {
        var promise = new Promise(function() {
            var xhr = new XMLHttpRequest(),
                url = this.baseUrl + "/add-achievement";

            ...
        }.bind(this));

        return promise;
    },

    findAll: function(limit) {
        var promise = new Promise(function() {
            var xhr = new XMLHttpRequest(),
                url = this.baseUrl + "/get-achievements";

            ...
        }.bind(this));

        return promise;
    }

};

And then to use this class:

var achievements = new AchievementsRepository("/ajax-member", 1);

achievements.findAll(10).then(function(data) {
    // All achievements fetched
}, function(error) {
    // Some sort of error occurred
});

achievements.add({
    foo: "bar",
    baz: "foo"
}).then(function(result) {
    // Achievement added successfully
}, function(error) {
    // There was a problem adding the achievement
});

This allows you to centralize all your AJAX calls, yet keep things strongly typed enough to make your code easy to follow.

Edit: This pseudo code uses the Promise/A+ API, which browsers have native support for now in ECMA Script 6. Related pollyfill: https://github.com/jakearchibald/es6-promise