Javascript code pattern

Question

I have been working on a javascript heavy project. There are sales, purchases, reports etc. What I've done is created a separate module for each i.e. a separate module for each i.e. a separate one for sale, one for purchase and so on. I am posting one of my modules, all the others have same structure. Can anyone please review it. Is there anything wrong with it or is everything okay. Any changes that I may want to consider to make it more better i.e. mantainable:

var navigation = {
init : function (){
 navigation.bindUI();
 $('.ReportViews').addClass('active');
 $('.stockNavReports').addClass('active');
},
bindUI : function () {
 $("#btnReset").on('click', function (){
 navigaiton.resetReport();
 });
 $("#btnShow").on('click', function (){
 var fromEl = $('#txtStart');
 var toEl = $('#txtEnd');
 var groupBy = navigation.getGroupingCriteria(); 
 if (navigation.validDateRange(fromEl.val(), toEl.val())) {
 navigation.fetchReportData(fromEl.val(), toEl.val(), groupBy);
 }
 else{
 fromEl.addClass('input-error');
 toEl.addClass('input-error');
 }
 });
},
getGroupingCriteria : function () {
 return $('input[name="grouping"]:checked').val().toLowerCase();
},
fetchReportData : function (startDate, endDate, groupBy) {
 if (typeof navigation.dTable != 'undefined') {
 navigation.dTable.fnDestroy();
 $('#navigationRows').empty();
 } 
 $.ajax({
 url: base_url + "index.php/report/getNavigationReportData", 
 data: { startDate : startDate, endDate : endDate, groupBy : groupBy },
 type: 'POST',
 dataType: 'JSON',
 beforeSend: function () {
 // console.log(this.data);
 },
 complete: function () { },
 success: function (result) {
 if (result.length !== 0) {
 $("#datatable_StockNavigationRows").fadeIn();
 var prevDate = "";
 var prevVrnoa = "";
 var prevGodown_1 = "";
 var prevGodown_2 = "";
 if (result.length != 0) {
 var navigationRows = $("#navigationRows");
 $.each(result, function (index, elem) {
 var obj = { };
 obj.SERIAL = navigationRows.find('tr').length+1;
 obj.VRNOA = elem.VRNOA;
 obj.REMARKS = (elem.REMARKS) ? elem.REMARKS : "Not Available";
 obj.QTY = (elem.QTY) ? elem.QTY : "Not Available";
 obj.GODOWN_1 = (elem.GODOWN_1) ? elem.GODOWN_1 : "Not Available";
 obj.GODOWN_2 = (elem.GODOWN_2) ? elem.GODOWN_2 : "Not Available";
 obj.VRDATE = (elem.VRDATE) ? elem.VRDATE.substring(0,10) : "Not Available";
 if (groupBy === 'date') {
 if (prevDate != obj.VRDATE) {
 // Create the heading for this new voucher. 
 var source = $("#report-dhead-template").html();
 var template = Handlebars.compile(source);
 var html = template(obj);
 navigationRows.append(html);
 // Reset the previous voucher to current voucher.
 prevDate = obj.VRDATE;
 }
 }
 else if (groupBy === 'vrnoa') {
 if (prevVrnoa != obj.VRNOA) {
 // Create the heading for this new voucher. 
 var source = $("#report-vhead-template").html();
 var template = Handlebars.compile(source);
 var html = template(obj);
 navigationRows.append(html);
 // Reset the previous voucher to current voucher.
 prevVrnoa = obj.VRNOA;
 }
 }
 else if (groupBy === 'godown_1') {
 if (prevGodown_1 != obj.GODOWN_1) {
 // Create the heading for this new voucher. 
 var source = $("#report-g1head-template").html();
 var template = Handlebars.compile(source);
 var html = template(obj);
 navigationRows.append(html);
 // Reset the previous voucher to current voucher.
 prevGodown_1 = obj.GODOWN_1;
 }
 }
 else if (groupBy === 'godown_2') {
 if (prevGodown_2 != obj.GODOWN_2) {
 // Create the heading for this new voucher. 
 var source = $("#report-g2head-template").html();
 var template = Handlebars.compile(source);
 var html = template(obj);
 navigationRows.append(html);
 // Reset the previous voucher to current voucher.
 prevGodown_2 = obj.GODOWN_2;
 }
 }
 // Add the item of the new voucher
 var source = $("#report-item-template").html();
 var template = Handlebars.compile(source);
 var html = template(obj);
 navigationRows.append(html);
 });
 }
 }
 navigation.bindGrid();
 },
 error: function (result) {
 //$("*").css("cursor", "auto");
 $("#loading").hide();
 alert("Error:" + result);
 }
 }); 
},
bindGrid : function() {
 // $("input[type=checkbox], input:radio, input:file").uniform();
 var dontSort = [];
 $('#datatable_StockNavigationRows thead th').each(function () {
 if ($(this).hasClass('no_sort')) {
 dontSort.push({ "bSortable": false });
 } else {
 dontSort.push(null);
 }
 });
 navigation.dTable = $('#datatable_StockNavigationRows').dataTable({
 // Uncomment, if datatable not working
 // "sDom": "<'row-fluid table_top_bar'<'span12'<'to_hide_phone' f>>>t<'row-fluid control-group full top' <'span4 to_hide_tablet'l><'span8 pagination'p>>",
 "sDom": "<'row-fluid table_top_bar'<'span12'<'to_hide_phone'<'row-fluid'<'span8' f>>>'<'pag_top' p> T>>t<'row-fluid control-group full top' <'span4 to_hide_tablet'l><'span8 pagination'p>>",
 "aaSorting": [[0, "asc"]],
 "bPaginate": true,
 "sPaginationType": "full_numbers",
 "bJQueryUI": false,
 "aoColumns": dontSort,
 "bSort": false,
 "iDisplayLength" : 100,
 "oTableTools": {
 "sSwfPath": "js/copy_cvs_xls_pdf.swf",
 "aButtons": [{ "sExtends": "print", "sButtonText": "Print Report", "sMessage" : "Stock Navigation Report" }]
 }
 });
 $.extend($.fn.dataTableExt.oStdClasses, {
 "s`": "dataTables_wrapper form-inline"
 });
},
validDateRange : function (from, to){
 if(Date.parse(from) > Date.parse(to)){
 return false
 }
 else{
 return true;
 }
}, 
resetReport : function (){
 $(".printBtn")
 $("#datatable_navigationRows").hide();
}
}
navigation.init();

• jfriend00: Do you intend for each module to use it's own global namespace object, rather than just have one master namespace object that each module could plug its own object into?
• Kamran Ahmed: I have placed each of the object in a separate file and each page that intends to use that object, loads the file and uses it.
• user229044: Your $.ajax success callback is way to big and cries out for refactoring.

Answer 1

In short,

your code looks good until $.ajax where far to many things happen, more specifically too many things happen in success.

The funny thing there is that it would look already a lot better if you took a generic approach to deal with the grouping.

Maybe you can have a groupingConfig object :

var groupingConfigs = 
{
 date : { template : '#report-dhead-template' , fieldName : 'VRDATE' },
 vrnoa : { template : '#report-vhead-template' , fieldName : 'VRNOA' },
 godown_1 : { template : '#report-g1head-template' , fieldName : 'GODOWN_1' },
 godown_2 : { template : '#report-g1head-template' , fieldName : 'GODOWN_1' }
}

then, before the $.each( you can get the groupingConfig

var groupingConfig = groupingConfigs[groupBy];

and within the $.each( you can simply

if (groupingConfig)
{
 if (previousValue != obj[groupingConfig.fieldName])
 {
 // Create the heading for this new group. 
 var source = $( groupingConfig.template ).html();
 var template = Handlebars.compile(source);
 var html = template(obj);
 navigationRows.append(html);
 //Reset the previous group value
 previousValue = obj[groupingConfig.fieldName];
 }
}

Also, the below creation of the dataTable looks all kind of wrong:

navigation.dTable = $('#datatable_StockNavigationRows').dataTable({
 // Uncomment, if datatable not working
 // "sDom": "<'row-fluid table_top_bar'<'span12'<'to_hide_phone' f>>>t<'row-fluid control-group full top' <'span4 to_hide_tablet'l><'span8 pagination'p>>",
 "sDom": "<'row-fluid table_top_bar'<'span12'<'to_hide_phone'<'row-fluid'<'span8' f>>>'<'pag_top' p> T>>t<'row-fluid control-group full top' <'span4 to_hide_tablet'l><'span8 pagination'p>>",
 "aaSorting": [[0, "asc"]],
 "bPaginate": true,
 "sPaginationType": "full_numbers",
 "bJQueryUI": false,
 "aoColumns": dontSort,
 "bSort": false,
 "iDisplayLength" : 100,
 "oTableTools": {
 "sSwfPath": "js/copy_cvs_xls_pdf.swf",
 "aButtons": [{ "sExtends": "print", "sButtonText": "Print Report", "sMessage" : "Stock Navigation Report" }]
 }
});

• Hungarian notation, don't do that
• sDom is a super long string without any comment
• the commented sDom has a superfluous comment
• bJQueryUI ?

Finally,

validDateRange : function (from, to){
 if(Date.parse(from) > Date.parse(to)){
 return false
 }
 else{
 return true;
 }
}, 

should really be

validDateRange : function (from, to)
{
 return !(Date.parse(from) > Date.parse(to));
},