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

• woolm110: thanks for the feedback @konijn, I'll look into making those amends you suggested, i've played with handlebars a little before so i'll see how i can integrate it into this :)

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