Code to update content via ajax

Following is the code to update content via ajax. Please review all aspects of the code.

```
var ajaxUpdate = {
init: function(){
ajaxUpdate.enableHistory();
},
_this:null,
enableHistory: function(){
History.Adapter.bind(window,'statechange',function() { // Note: We are using statechange instead of popstate
var State = History.getState();

console.log('History: '+State.url)
if(State.data.type == 'maincontainer'){
if(jQuery('#maincontainer').length){
//update content
jQuery('#maincontainer').html(State.data.response);

//select the menu element
jQuery('#main-menu li').removeClass('active');
jQuery('#main-menu li:eq('+State.data.main+')').addClass('active');
}
else{
window.location.href = State.url;
}
}/*
_this.parent().addClass('active');*/
});
},
updateMainContent: function(){
var _this = ajaxUpdate._this;
if ( !History.enabled ) {
return ;
}

jQuery.ajax({
type: 'GET',
url: _this.attr('href'),
beforeSend : function(jqXHR, settings){
jQuery('#maincontainer').addClass('loading');
},
complete: function(jqXHR, textStatus){
jQuery('#maincontainer').removeClass('loading');
},
success: function(response){
var data = {
main : _this.data('main_index'),
type: "maincontainer",
response: response
}
jQuery('#maincontainer').removeClass('loading');
History.pushState(data, _this.data('title'), this.url);
},
error : function(jqXHR, textStatus, errorThrown){
//processERROR();

A couple things right off the bat:

-
You use jQuery('#maincontainer') multiple times. You should consider caching that somewhere. Either as part of your object or a separate variable.

// Wherever you use jQuery('#maincontainer'), use ajaxUpdate.mainContainer
ajaxUpdate.mainContainer = jQuery('#maincontainer');

-
I would suggest passing the this from your event handlers directly to updateMainContent since that is the only method that uses it.

-
Unless you have plans to significantly change the way events are handled between search, submenu, and mainmenu, you can can just have one method that handles those events.

-
complete gets called whether it's an error or success. No need to remove the loading class on success and complete.

-
Your beforeSend function is unnecessary since it doesn't actually manipulate any of the data before the AJAX call or cancel the AJAX call. You can move the addClass to before the jQuery.ajax. From the jQuery docs:


A pre-request callback function that can be used to modify the jqXHR (in jQuery 1.4.x,
XMLHTTPRequest) object before it is sent. Use this to set custom headers, etc. ...
Returning false in the beforeSend function will cancel the request.

-
You can chain the following lines together (to remove another call to jQuery):

jQuery('#main-menu li').removeClass('active');
jQuery('#main-menu li:eq('+State.data.main+')').addClass('active');

Like so:

jQuery('#main-menu li').removeClass('active')
    .filter(':eq('+State.data.main+')').addClass('active');

-
Switching your AJAX call to a .get and using the promise API cleans up the code a little and makes it a bit more concise.

-
In your success response, you don't need to define data since you're just passing it directly to History.pushstate.

Not knowing the HTML structure, I think that's the best I can give you. Here's a fiddle with all of these things implemented.

I'm more than happy to answer any questions if you have them. Hopefully this will help you out.

StackExchange Code Review Q#29302, answer score: 5