Comments on a file that needs correction

order-summary.js

define([
    'jquery',
    'app/controllers/modals/view-transaction-details',
    'app/controllers/modals/summary-member-savings',
    'app/controllers/modals/summary-member-points',
    'app/controllers/modals/promotional-new'
], function(
	$,
	TransactionDetails,
	SummaryMemberSavings,
	SummaryMemberPoints,
	PromotionalNewModal) {

	var self = {};
	
	self = {
		orderSummaryTemplate: null,

		init: function(){
			
			PromotionalNewModal.init(self);
			
			// this just needs to check to see if its truthy
			if($('.order-summary-wrapper').html() != undefined){
				self.attachListeners();
				self.displayOrderSummary(window.orderSummary);
				// this would execute a message on every page. dont leave these in unless they're needed absolutely
				console.log('ORDER SUMMARY');
			}
		},

		// all listeners in the attachListeners list should be calling methods that are named, not anonymous closure functions
		attachListeners: function () {
			// this should not be calling an anonymous function
			// this should not be using a live listener
			$(document).on('tap', '#view-item-details', function() {
				
				PromotionalNewModal.showModal();
				
				$.ajax({
					type:"GET",
					url: "viewdetails.htm",
					success: function(data){	
					    self.viewCartSuccess(data);
					},
					error : function(e) {
						console.log(e);
						PromotionalNewModal.hideModal();
					}
				});
			});
			
			// this should not be calling an anonymous function
			// this should not be using a live listener
			// this should not be calling an unscoped listener on all customClose buttons in a generic way
			$(document).on('tap', '.customClose', function(e) {
				e.preventDefault();
				$(this).parents(".modal").jqmHide();
			});
			
			// this is a possible memory leak. this function should also be attached to this object, not anonymous
			// this should not be using a live listener
			$(document).on('tap', '.js-order-summary-modal-trigger', function(e) {
				e.preventDefault();
				// This should not be dynamically calling modals based on a data attribute. know what you're trying to pop up. add code clarity!
				var $modal = $(this).attr('data-modal');
				$('#' + $modal)
					.jqmShow()
					.trigger('create'); // this isnt even how we initialize modals. please use the requirejs way for initializing jqm windows
				
				// this should not be creating new iscrolls on every click. this is going to stack up memory from new objects being spawned over and over
				var scroll;
				scroll = new iScroll('scroller-'+ $modal, {
					hideScrollbar : false,
					useTransition : true
				});
				
				// its unlikely that we need to set a timeout here. 
				setTimeout(function() {
					scroll.refresh();
				}, 200);
				
			});
			
			// this should not be calling an anonymous function
			// this should not be using a live listener
			$(document).on('tap', '#close-transaction-details', function() {
				// call this with the require module. 
				// this will also allow you to bind the tap handler right to the require module so the event can be passed to it
				$("#view-cart-modal").jqmHide();
			});
			
			// this should not be calling an anonymous function
			// this should not be using a live listener
			$(document).on('tap', '#back-to-cart', function() {
				
				// call this with the require module, not with a selector
				$('#view-cart-modal').jqmHide();
				var fromPageNew = $('#fromPageNew').val();
				
				// use strict comparisons
				if(fromPageNew=='sywr') {
					window.location = "removeSywrPaymentCheck.htm?landingPage=cart";
				// use strict comparisons
				} else if(fromPageNew=='payment') {
					$.ajax({
						type:"GET",
						url: "unpairDevice.htm",
						success: unpairDeviceSuccess,
						error : function(e) {
							console.log(e);
						}
					});
					window.location = "removePaymentCheck.htm?landingPage=cart";
				} else {
					window.location = "viewCart.htm";
				}
			});
		},
		
		renderOrderSummary: function(template, ordSummary){
			
			self.renderOrderSummaryModals(ordSummary);				
			var fromOrderComplete=false;
			
			// in most instances, you do not need to check to see if something is undefined. 
			// if the variable instance is initialized to null or undefined, such as it is with the return from this function, 
			// you can just check to see if it is truthy 
			if($('#fromPageNew').val()!=undefined 
					&& $('#fromPageNew').val()=='orderComplete'){
				fromOrderComplete=true;
			}
			
			if(!fromOrderComplete){
				var summary = {	data: ordSummary };
				// calls to access properties of objects should use dot syntax. Only dynamic reads should use brackets (code clarity)
				summary['config'] = config;
				// here we're calling a global instance of _template instead of properly using the Mustache require module, so we're breaking modularity
				$orderSummaryHtml = $(window._template(template, summary));
				
				// use strict comparisons
				if(document.title == 'Cart'){
					$orderSummaryHtml.find('.view').hide();
				}
				
				// use strict comparisons
				if(document.title == 'Cart' || document.title == 'Coupons & Discounts'){
					$orderSummaryHtml.find('h2').text('Summary');
					$orderSummaryHtml.find('.amount_due').hide();
				}
				
				// use strict comparisons
				if(document.title == 'Checkout'){
					$orderSummaryHtml.find('.amount_due').hide();
				}
				
				if($(".order-summary-wrapper").find("#order-summary-template")){
					$("#order-summary-template").html("");
				}
				
				// again, most times you dont need to check to see if its undefined, just truthy
				if($('#installNextButton').html() != undefined){
					$(".order-summary-wrapper").prepend($('#installNextButton').html());
				}
				
				// what is trigger('create') doing?
				$(".order-summary-wrapper")
					.prepend($orderSummaryHtml)
					.trigger('create');
			}
		},
		
		displayOrderSummary: function(ordSummary) {
			if (!self.orderSummaryTemplate){
				$.ajax({
					type: "GET",
					url: "html/order_summary.html",
					success: function (template) {
						self.orderSummaryTemplate = template;
						self.renderOrderSummary(self.orderSummaryTemplate, ordSummary);
					}
				});
			}else{
				self.renderOrderSummary(self.orderSummaryTemplate, ordSummary);
			}
		},
		
		viewCartSuccess:function (data){
			// mustache templates can and should be imported through require's dependency list at the top of the file. 
			// this will allow them to be bundled with the js and limit the amount of code you need to use them. 
			// it will also optimize calls in other files for those templates should they need them loaded too.
			$.ajax({
				type: "GET",
				url: "templates/viewCart.mustache",
				cache:false,
				success: function(templateHTML){					
					self.viewCartTempHandler(templateHTML, data);
				}
			});	
		},
		
		viewCartTempHandler:function(template,data){
			var strTemplates = $(template),
				temp = $.grep(strTemplates, function(e){ return e.id === "viewCartTemplate"; })[0].innerHTML,
				html = Mustache.to_html(temp,data['result']), // this should be using dot notation
				$viewCartDetail = $("#viewCartDetail");
			
			$viewCartDetail.html(html);
		 	TransactionDetails.init();
		 	// this should be called from a require module, not by a jquery selector. we're in require!!! all modals have their selectors cached
		 	$("#view-cart-modal").jqmShow();
		 	$viewCartDetail.trigger('create');
		 	
		 	// string comparisons should be done using strict comparison operators so no conversion is taking place. this is a performance thing
		 	if(document.title == 'SYWR Redemption' || document.title == 'Payment'){
		 		$viewCartDetail.find('#back-to-cart').hide();
		 		$viewCartDetail.find('#close-transaction-details').hide();
		 		$viewCartDetail.find('#close-transaction-details-cross').show();
			}
		 	
		 	// This is a possible memory leak
		 	var scroll;
			scroll = new iScroll('scroller-content-cart', {
				hideScrollbar : false,
				useTransition : true
			});
			
			// its unlikely we need to do a timeout on this refresh
			setTimeout(function() {
				scroll.refresh();
			}, 200);

			PromotionalNewModal.hideModal();
		},
		
		// mustache files should be loaded using the require dependency list
		renderOrderSummaryModals:function (orderSummary){
			$.ajax({
				type: "GET",
				url: "templates/orderSummaryModals.mustache",
				cache:false,
				success: function(templateHTML){					
					self.orderSummaryModalsTempHandler(templateHTML, orderSummary);
				}
			});	
		},
		
		orderSummaryModalsTempHandler:function(template,orderSummary){
			// multi line var starts here
			var strTemplates = $(template),
				temp = $.grep(strTemplates, function(e){ return e.id === "orderSummaryTemplate"; })[0].innerHTML;
			// but then abruptly ends and starts again here
			var html = Mustache.to_html(temp,orderSummary); 
		 	$("#order-summary-modals").html(html);
		 	// and then starts again here... WHY?!
		 	var totalPoints = $("#totalPoints").val(),
		 		subtotal = $("#subtotalPoints").val(),
		 		coupons = $("#couponsPoints").val();

		 	// just do a truthy check
		 	if(totalPoints != "" || subtotal!="" || coupons!="" ){
		 		$('#earned_subtotal').append(self.commaSeparateNumber(subtotal));
		 		$('#earned_coupons').append(self.commaSeparateNumber(coupons));
		 		$('#earned_total').append(self.commaSeparateNumber(totalPoints));
		 	}
		 	
		 	// what is this triggering? a modal? multiple modals?
		 	$("#order-summary-modals").trigger("create");
			SummaryMemberSavings.init();
			SummaryMemberPoints.init();
		},
		
		// is this a utility method that could be abstracted out to the globals include or a utility math class?
		commaSeparateNumber : function(val){
	 	    while (/(\d+)(\d{3})/.test(val.toString())){
	 	      val = val.toString().replace(/(\d+)(\d{3})/, '$1'+','+'$2');
	 	    }
	 	    return val;
	 	}
	};
	
	return self;
});