JavaScript – Web Development Standards

The purpose of this document is to make the JavaScript (JS) code produced by all development teams more predictable, consistent, and easier to read.
This is not an introduction to how to write JS or how it works. This document outlines best practices for formatting, naming conventions, and utility code.


JavaScript Specifications

JavaScript files should be added to the page in a consistent manner, at the bottom of the page, if possible.

  • As of this writing, assume that SHC is using JavaScript 1.5
  • The “language” attribute is obsolete and should not be used.
  • The type=”text/javascript” attribute is required.
  • The closing </script> tag is required.
  • Script tags in the HEAD element must follow any link tags which load CSS files
  • Minimize/eliminate the use of inline script blocks. Page loading can be delayed while they are parsed. They are harder to debug and can’t be compressed

Example:

<script src="scripts/scriptName.js" type="text/javascript"></script>

Coding Conventions

There are a number of coding conventions established to make maintenance easier and eliminate potential bugs. Proper use of these will make code easier to read and maintain, reduce hard to find errors, and eliminate some possible bugs that might only come up when the code is compressed.

Variable/Function naming and declaration

  • Do not use overly verbose names. Long names waste bandwidth and make the code harder to read. If you need long names to understand the code, the code is probably too complex and in need of refactoring.
  • Variable names should be camel-case unless it’s a short acronym.
  • Do not preface variable names with a ‘$’. The ‘$’ should only be used as shortcut to the jQuery object.
  • Do not use Hungarian notation when naming variables. The exception to this would be property names in an object. For example, if you have multiple button objects, it’s helpful to name them btnXXX, btnYYY, btnZZZ. This way they will appear alphabetically together in the debugger’s object view.
  • Don’t use underscores and caps unless you’re declaring a constant.
  • Function names should be camel-case.
  • Constructor functions should be camel-case, but start with a capital letter to distinguish them from regular functions.
  • Do not prefix function names with ‘fn’.
Example:

var intCount = 0, strName = '';							// incorrect Hungarian notation
var lazy_RED_FOX;										// not camel case
var searsAddr = '333 Beverly Rd, Hoffman Estates';		// constant
var count = 0, name = '', lazyRedFox,					// correct
	SEARS_ADDR = '333 Beverly Rd, Hoffman Estates';

Variable scope

Unlike C#/Java, JS does not use block scope. Declare variables at the top of the function, rather than scattered throughout the function.
It is no longer recommended to declare the index variable (and the length variable if used) in a ‘for’ loop as part of the loop. That variable should be declared at the top of the function with the other variables.
This is now a breaking error in JSlint, and coding it the wrong way will make it difficult to perform a complete scan of the code.

Example:

//old way
var foo = ['a', 'b', 'c'];

for (var i=0, len=foo.length; i<len; i++) {
	alert(foo[i]);
}

//new way
var foo = ['a', 'b', 'c'], i, len;

for (i=0, len=foo.length; i<len; i++) {
	alert(foo[i]);
}

The use of the ‘var’ keyword is not required by JS, however omitting it will create a global variable, regardless of whether it is inside or outside a function block.
Always use the ‘var’ keyword for variable declarations. It is also better from a performance perspective to code a single ‘var’ statement, separating the names with commas, rather than coding multiple ‘var’ statements.


//incorrect
var foo;
var bar = 1;
var baz = true;
var x;

//correct
var foo, x,
	bar = 1,
	baz = true;

Use of semicolons, braces, parentheses, quotes, tabs, and carriage returns

  • Terminate all statements with a semicolon. Do not assume a carriage return ends the statement.
  • Opening braces appear on the same line, preceded by a single space, rather than on a newline (unlike C#).
  • Closing braces appear on a newline, followed by a carriage return.
  • Opening parens in conditional expressions are preceded by space, but not followed by a space.
  • Closing parens in conditional expressions are followed by a space, but not preceded by a space.
  • Braces and carriage returns should always be used around conditional statements, even if there is only 1 statement. e.g. if-else, for loops, etc.
  • A single space should follow arguments in a function call as well as in the function declaration
  • Use single quotes around JavaScript literals. Use double quotes around HTML attributes.
  • When editing, use tabs rather than spaces. Set your editor tab stops/indents to 4 characters. Do not convert tabs to spaces upon saving (configuring this will depend upon your editor).

The following is both difficult to read and has the potential to introduce bugs.


if(n <=0) return '';
else if(n > this.length) return this
else return this.substring(0,n);

It should be written as:


if (n <= 0) {
	return '';
}
else if (n > this.length) {
	return this;
}
else {
	return this.substring(0, n);
}

Accessing DOM elements

Use the jQuery ‘$’ function instead of document.getElementById. This has multiple benefits:

  • Shorter string means fewer bytes sent to client.
  • Less likely to introduce bugs due to incorrect case of getElementById
  • Single entry points allows for future optimization of this call e.g. element caching
  • Using the jQuery $ function allows you to chain operations on a single DOM element.

Proper use of == vs === and != vs !==

JS allows values to be compared in several ways. Comparison with 2 equal signs (==) forces type coercion to do the comparison. Comparison with 3 equal signs (===) requires that the types must match also.

Example:

var a=1, b='1';
if (a == b) {
 // true b/c type is not considered
}
If (a === b) {
// false because a is numeric but b is string
}

Whenever possible you should use === for comparision. Because JS doesn’t have to do type coercion, the code will run faster. The same rules apply to checking for items that are not equal.
This issue is now flagged by JSLint and you can get 100s of warnings for this, making it difficult to see real errors.

Note: One place to be very careful about implementing this is checks for storeId. Although storeId looks like a string (e.g. ‘10153), there are places is in the server-injected code where it’s set as a number and your code could fail mysteriously because the types don’t match. For now, it’s safer to compare storeId using ==.

Proper use of typeOf and delete

typeof and delete are operatiors, not functions. Do not use parentheses around their arguments.
The result of typeOf is a string and comparsion should always be done with === or !==.
The delete operator is only useful for deleting properties of an object. It does nothing when called on a simple variable.


if (typeof(foo) === 'undefined') {
	delete (foo.bar);
}
//should be
if (typeof foo === 'undefined') {
	delete foo.bar;
}

var i=0, foo={x:1, y:2};
delete i; //this does nothing
delete foo; //neither does this

Use parentheses only when needed to clarify operational order, but be mindful of the effects of code compression

The following is an example of poor coding that would result in very hard to find bug, usually only appearing in a production environment, when compression is enabled.


var next = $(this._mainSection + '_'{color:red} + ++this._pageIndex);

When the above code is compressed, and spaces removed, the line becomes this:


var next=$(this._mainSection+'_'{color:red}+++this._pageIndex);

It is now indeterminate as to what is being incremented vs what is concatenated, especially when the quoted string is numeric. This should be written as


var next = $(this._mainSection + '_' + (++this._pageIndex));

Always use the second argument on parseInt calls

Calls to the parseInt function without specifiying ’10’ as the second argument cause the function to operate in base8 (octal mode) when the first character of the first argument is a ‘0’, which can create hard to find bugs. If you aren’t familiar with why this is an issue, consider the following code.


var p1a = parseInt('7');        //returns 7
var p1b = parseInt('07');       //returns 7
var p1c = parseInt('07', 10);   //returns 7
var p2a = parseInt('8');        //returns 8
var p2b = parseInt('08');       //returns 0 **** BAD!! ****
var p2c = parseInt('08', 10);   //returns 8

Depending on the test condition, sometimes you’ll get the right answer, sometimes you won’t. This is also the cause of the cryptic JSLint ‘missing radix’ message.

Minimize the use of global variables and functions

Variables in JS either have global scope or function-level scope, and using the ‘var’ keyword is vital to keeping them straight. As a best practice, please refrain from using a large amount of global variables.

The example below highlights the potential problem caused by not doing so.


// bad way
var i=0;
function test() {
   for (i=0; i < 10; i++) {
	 alert("this should be kept to a minimum");
   }
 }

// better way
function test() {
  var i=0;
  for (i=0; i < 10; i++) {
	   alert("Im happy! Cleaner Code");
  }
}

Why should we keep this to a minimum?

  • Naming collisions are possible, especially when trying to implement third party JS libraries.
  • If the variable ‘i’ is used anywhere else, it will contain the value from the call to ‘test’ if it’s called, but will be 0 if ‘test’ is not called.
  • Debugging is difficult because determining where a variable may have been accessed/changed requires searching all the code.
  • The code will run slower because JS has to search the scope chain to find ‘i’
  • Since everything is global, all variables and functions are displayed in one huge list in the debugger. There’s no way to group them by object construct and collapse/hide the ones you don’t need to see.

Use the standard jQuery function to make AJAX calls

Note:All other existing functions are obsolete, including ajaxCall and searsAjax.
Follow standard formatting for data passed to the call in the following order:


$.ajax({
  url: 'SomeAjaxCmd',
  data: 'storeId=' + storeId + '&catalogId=' + catalogId,
  success: function(res) {
	// always trim an html response to avoid injecting JSP generated whitespace into DOM
	res = trim(res);
  },
  error: function() {
   // the error handler is not required and may be omitted if not used
  }
});
Parameter Details:
  • url: A string containing the URL to which the request is sent.
  • data: Data to be sent to the server. It is converted to a query string, if not already a string. It’s appended to the url for GET-requests.
  • type: Defaults to ‘GET’. Should be omitted unless you’re doing a POST operation
  • dataType: Defaults to ‘text’
  • Refer to the current jQuery documentation for additional parameters

If your success handler function is large, do not stuff it all inline in the $.ajax call. Create a separate function and pass the function as the property for ‘success’.

If you need to pass additional arguments to the success handler function, use createDelegate. There are many examples in the codebase that illustrate how to do this, in addition to the documentation of createDelegate in NewUtility.js.

Note:NewUtility contains helper functions to handle 2 special cases of AJAX requests. See the NewUtility doc for complete details.

  • FED.Util.addCssFile – add a LINK tag to retrieve a CSS file
  • FED.Util.addJsFile – add a SCRIPT tag without a cache-buster parm
    This should be used instead of $.getScript or $.axax with a dataType:’script’, to allow caching of js file requests.

Eliminate unnecessary quotation marks in object literals


var obj = {
	'foo': 'bar',
	'x': 1
};

The quotes around foo and x are unnecessary and waste bandwidth. The only time quotes would be needed in this case is if the property name was a JavaScript reserved word. This should be avoided at all costs to eliminate potential bugs due to omission of the quotes somewhere else in the code. Never use reserved words as property names!

Note:The above recommendation may not apply if you are using built-in browser functionality that expects quotes around property names. This is usually the case when decoding a JSON response from the server.

Use standard methods to do browser sniffing.

Do not check DOM properties to determine browser version (e.g. document.all).
While feature detection may be a recommended way to do this instead, SHC currently is not using any advanced features that warrant this, and we’re better off following a simple standard.
Note:document.all was introduced by Microsoft in IE and is not a standard JavaScript DOM feature.
Use the existing jQuery properties or more simply you can now test the browser class which is on every page (built by NewUtility.js). These include ie, ie7, ie8, ie9, ie10, gecko, gecko3, safari, safari3, safari4, opera, chrome.


//these 3 are the same, however, $.browser is now deprecated in jQuery.
if ($.browser.msie && parseInt($.browser.version,10) == 7) {...}
if ($('body').is('.ie7')) {...}
if ($('body').hasClass('ie7') {...}
Note:Read the NewUtility section for more info on NewUtility.js.

Use the “switch” statement to handle multiple conditions.

It’s easier to read and may run marginally faster in some browsers.


//bad
if (color === 'cyan') {
	// do something here...
} else if (color === 'magenta') {
	// do something here...
} else if (color === 'yellow') {
	// do something here...
} else if (color === 'black') {
	// do something here...
} else {
	// do something here...
}

//better
switch (color) {
  case 'cyan':
   // do something here...
   break;
  case 'magenta':
   // do something here...
   break;
  case 'yellow':
   // do something here...
   break;
  case 'black':
   // do something here...
   break;
  default:
   // do something here...
  break; }

Don’t change a variable’s type after initial declaration

In JS, technically this is allowed and legal.
After the variable’s initial declaration as a string on line 1, on line 2 its value changed and its data type changed. Even though this allowed, it’s not good practice and should be avoided at all times.


var my_variable = "This is a String";
my_variable = 50;

Do not use ‘@cc_on’ style comments.

For example, this code is buried in numerous js files across the sites.


function replaceHtml(el, html) {
	var oldEl = typeof el === 'string' ? document.getElementById(el) : el;
	/*@cc_on // Pure innerHTML is slightly faster in IE
		oldEl.innerHTML = html;
		return oldEl;
	@*/
	var newEl = oldEl.cloneNode(false);
	newEl.innerHTML = html;
	oldEl.parentNode.replaceChild(newEl, oldEl);
	/* Since we just removed the old element from the DOM, return a reference
	to the new element, which can be used to restore variable references. */
	return newEl;
};

This relies on IE browser bugs (similar to CSS hacks) to only apply lines within the comment block to certain conditions (e.g. IE or Firefox).

It’s not very clear at first glance what a specific comment may be doing and it deviates from all accepted standards of browser sniffing. The biggest problem is that it relies on JavaScript comment blocks which may be stripped out during compression. That means the app is immediately broken and the error won’t necessarily be a hard error. You will only see the error if you happen to be testing in the correct browser at the place where the comment used to be. If you’re not familiar with a specific test case, you might not even notice that there is a difference in behavior from one browser to the next.


Use Common Utility functions from NewUtility.js

NewUtility.js contains a set of common utility functions – these are not specific to any business logic, but rather are used to perform common functions like string trimming, numeric validation, cookie access, etc. This code is included on all pages and these functions should be used in favor of any previously defined utility functions.

In order to provide backward compatibility, all existing global functions previously defined in other files are aliased here to the new function names. For example, NewUtility.js contains functions called ltrim and rtrim. The other global code contains multiple versions of these with different spelling (ltrim, Ltrim, LTRim), not necessarily with the same internal implementation. This is handled by NewUtility.js as illustrated below (all old function definitions are in the process of being removed).


 /**
 * Return string with leading spaces removed.   Normally you should just call String.ltrim, but if there's a
 * chance that the string could be null, this function will handle it.
 * @returns the trimmed string.
 */
function ltrim(s) {
	return typeof s != 'undefined' && s != null ? s.ltrim() : '';
}
//back-compat aliases
Ltrim = ltrim;
LTRim = ltrim;

When this code loads it also modifies the class attribute of the BODY tag. It adds classes to indicate what browser is being run. For example you might see:

  • class=”gecko gecko3″ indicating that the user is running Firefox3.
  • class=”ie ie7″ indicating that the user is running IE7.
  • class=”safari mac” indicating that the user is running Safari on a Mac.

Note:See the NewUtility page for the complete API documenation

Use standard comment structure to document functions.

Using the proper formatting of comments on top of a function definition allows us to fully understand the purpose behind each function. Make it as long and as detailed as possible, minification removes all of the comments so it does not matter how bloated the code may seem. There is only one exception to this, if JS/jquery is written in a dropzone then please be short and straight to the point. All comment blocks for function should be formatted as follows:


/**
 * Replace all instances of one string within another string.
 * @param  s the string to search within.
 * @param  find the string to find.
 * @param  replace the string to replace with.
 * @returns the modified string.
 */
function replaceAll(s, find, replace) {...}

Note that the function definition immediately follows the comments with no blank lines and you do not wrap the comments in a huge string of ‘***’.

See NewUtility.js which is fully commented for further examples as well as additional ‘@’ tags which may be used. For more information on JavaDoc style comments, refer http://en.wikipedia.org/wiki/Javadoc

JavaScript Tools

There are a number of recommended tools to assist in writing and debugging JS code


Use JSLint to check your code

The JavaScript parser / validator at jslint.com is very helpful for pointing out problems with your code. Following the above guidelines will reduce the number of errors/warnings that JSLint produces, so that you can more easily find the serious problems.
Run your code thru jslint.com before compressing to look for errors.


 

Compress JavaScript

JavaScript is compressed using the YUI Compresssor This is same compressor used in the batch process to deploy production code.
Note:This supercedes the previous recommendation to use the JSMin Compressor


 

Use Firebug to debug your code

Use the Firebug addon for Firefox or the IE debugger in IE8+ to debug your code. Debugging via the addition of numerous alert or console statements is not productive.

Performance “Do’s and Don’ts”

As the site grows more interactive and complex (some might argue it is already exhaustingly so), a solution to a problem may cause more problems than it solves unless Performance considerations are taken. This applies to deliverables as well as hotfixes we sometimes implement.
As many JS programmers just ‘make it work’ rather than learn the language, performance has become an unintended victim of quick solutions.
For example, we’ll look at using a class selector rather than an id selector. Thanks to the Sizzle selector engine in jQuery this is something that can be done, but should not be done for several reasons.

  • JS has a native getElementById function in all browsers that jQuery uses. Some browsers natively support getElementByClassName, but not IE. This means jQuery needs to resort to a slower method to search by classname
  • since there can be multiple classes in a document where only one ID is permitted, a function searching for a class selector alone will check every single node in the DOM.

String concatenation

String concatenation can be optimized by using Array.join. Using join is faster and more memory efficient than concatenation. There’s no hard and fast cutoff here, but it’s recommended that if you have 5 or more strings to concatenate, use join.

For example:


var s = 'The lazy ';
s+= 'brown fox ';
s+= 'the sleeping ';
s+= 'red dog ';
s+= 'on the ';
s+= 'back porch';
//Should be written as:
var arr = ['The lazy','brown fox', 'the sleeping', 'red dog', 'on the', 'back porch'],
	s = arr.join(' ');
//or
var s, arr = [];
arr.push('The lazy');
arr.push('brown fox');
arr.push('the sleeping');
arr.push('red dog');
arr.push('on the');
arr.push('back porch');

s = arr.join(' ');

You can also use the format function which has been added to the String object to avoid concatenation.
You can pass as many parameters as needed, starting with 0, 1, 2… Internally, this also uses the array join technique.


// returns 'Street:123 Oak St, City:Chicago, State:IL, Zip:60060'
var s = String.format('Street:{0}, City:{1}, State:{2}, Zip:{3}', '123 Oak St', 'Chicago', 'IL', '60060');

This is especially useful in cases where you’re building HTML strings and need to deal with nesting of quotes.


// returns '<div id="foo" class="bar">The lazy red fox</div>'
var html = String.format('<div id="{0}" class="{1}">{2}<<div>, '<foo', 'bar', 'The lazy red fox');

Note:As browser engines become better at optimizing code, this is becoming less of an issue. However, if string concatenation is being done without using arrays, the following is the preferred way to do it.


//do not needlessly assign blank to s before starting the string building
//and always put the '+' sign at the end of line, not the beginning.
var s = 'The lazy +
	'brown fox ' +
	'the sleeping ' +
	'red dog ' +
	'on the ' +
	'back porch';

Accessing DOM elements

    • Do not look up the same DOM element multiple times. In a large page this is a big performance issue. Use jQuery chaining or assign the value to a local var.
    • Retrieve elements by ID rather than class whenever possible to improve performance. When you must find elements by class name, limit the scope of the search to a parent container ID.

$("#parentID .childClass") vs $(".childClass")

Eliminate the use of new Array() and new Object()

There is no benefit to using the constructor versions to create arrays/objects, rather than simply using [] and {}. In addition to reducing code size, there is also a slight performance gain by eliminating the constructor call.


var arr = new Array(), obj = new Object(); // Wrong
var arr = [], obj = {}; // correct

Adventures in bad coding

This section highlights some examples of real world bad coding found on Sears and myGofer and how they might be handled differently. The point is not to just say how bad the code is, but to illustrate how to do things better. In some cases this code could also benefit from being able to select by ID rather than class name.

#1 from productLayoutOptions.js


	function displayQuestions(childId,parentId){
	//For Adding installation to order summary when clicking one of the installation base option.
	var	instDiv = '';
	var	elementsArray = '';
	var	radioButton;
	var	totCurrInstPrice = 0;
	var	divId = parentId+'_installation_options';
	var	priceTemp='';
	if(null !=document.getElementById(divId)){
		instDiv =document.getElementById(divId);
		if(null !=document.getElementById('input')){
			elementArray =instDiv.getElementByTagName('input');
		}
		var	count;
		for(count = 0; count < elementsArray.length; count++)
		{
			if(elementsArray[count].type=='radio')
			{
				radioButton=elementsArray[count];
				if(radioButton.checked==true){
					var	childId=radioButton.id.split('_')[2];
					var	div =document.getElementById(childId+'_'+parentId);
					var checkBoxed=div.getElementByTagName('input');
					var count2;
					var	priceTemp;
					for(count2 = 0; count2 < checkBoxes.length; count2++)
					{
						if(ch3ckBoxes[count2].type=='checkbox')
						{
							if(checkBoxes[count2].checked==true)
							{
				priceTemp = checkBoxes[count2].value;
				priceTemp = parseFloat(priceTemp);
				totCurrInstPrice = totCurrInstPrice + priceTemp;
							}
						}
					}
				}
			}
		}
	var	baseInstPrice=($('#'+parentId+'_'+childId+'_total_cost').text()).substring(1);
	$('#optInstall1').val(baseInstPrice);
	addOptions('SEARS Professional Installation');
}

The first thing we see is no use of jQuery and multiple lookups in the DOM for the same element. First to see if it exists, then get a reference to it, then find children by tag name. Multiple vars are created and assigned for only a single use. childId is passed into the function as an argument and declared as a local inside the loop. The value of childId is then used after loop again. This entire block could be re-written as follows.


//For Adding installation option to order summary when clicking one of the installation base option.
var	totCurrInstPrice = 0;
$('#'+parentId+'_installation_options input[type=radio]').each(function(){
if(this.checked){
childId = radioButton.id.split('_')[2];
$('#'+childId+'_'+parentId+'input[type=checkbox').each(function(){
	if (this.checked){
	totCurrInstPrice += parseFloat(this.value)
	}
});
}
});

The code above could be further refined so that you don’t recreate the function for the inner ‘each’ for every time thru the outer ‘each’. The only problem with all of this refactoring is that after doing it, you realize that the whole point is simply to calc the value of ‘totCurrInstPrice’, which is never used after calculation. The entire routine can be removed.

#2 from productLayoutOptions.js


	if ($('.first_option span').text().split('$')[2] == null || $('.first_option span').text().split('$')[2] == 'undefined' ||
	  $('.first_option span').text().split('$')[2] == "") {
	  $('#optInstall1').val(($('.first_option span').text()).substring(1));
	}
	else {
	  $('#optInstall1').val(0);
	}

How many times can you access the same element (by classname) and split the array and look at the same element?

The re-written code eliminates all the extra DOM lookups and calls to text() and splitting the array. It also reverses the condition to make use of the double-exclamation null/empty check and then use an inline ‘if’.
This code should probably be code a little more defensively to make sure that the array actually has at least 3 items after splitting.


	var opt = $('.first_option span').text(),
		item = opt.split('$')[2];
				 
	$('#optInstall1').val((!!item && item != 'undefined') ? '0' : opt.substring(1));

#3 from s_code_combined.js


	if ($('#pricingWrap').find('div.mapdesc').children().hasClass("mapTwo") ||
		$('#pricingWrap').find('div.mapdesc').children().hasClass("mapFive") ||
		$('#pricingWrap').find('span.setSeven').length ||
		$('#pricingWrap').find('div.mapdesc').children().hasClass("mapEight")) {
		s.eVar63 = 'MAP CO';
	} else if ($('#pricingWrap').find('div.mapdesc').children().hasClass("mapOne") ||
		$('#pricingWrap').find('div.mapdesc').children().hasClass("mapThree") ||
		$('#pricingWrap').find('div.mapdesc').children().hasClass("mapFour") ||
		$('#pricingWrap').find('div.mapdesc').children().hasClass("mapSix")) {
		s.eVar63 = 'MAP Cart';
	}

How many times do you really need to run an ugly selector to check for existence of an element with a given class? In the worst case, you would have done a look on the same element 7 times to find ‘mapSix’.

Caching the parent selector and calling children with multiple candidate classes, we can streamline this to


	var el = $('#pricingWrap').find('div.mapdesc'); 
	if (el.children('.mapTwo, .mapFive, .mapEight').length || $('#pricingWrap').find('span.setSeven').length) {
		s.eVar63 = 'MAP CO';
	} else if (el.children('.mapOne, .mapThree, .mapFour, .mapSix').length) {
		s.eVar63 = 'MAP Cart';
	}

#4 from s_code_combined.js


function getMemberStatus(){
	var raIdCookie = "";
	var raCookie = "";
	var memberStatus = 'Anonymous';
	var craftsmanClubMember = "";
	var isAssociate = "";
	var sywrMember = "";
	var shipVantageMember = "";
	var sywingo=$.readUpdateJsonCookie('c_i','sywingo');

	if(!!sywingoJson){
		sywingo = sywingoJson.sY;
	}

    if ($.cookie("ra_id") != null) {
		raIdCookie = $.cookie("ra_id");
		raCookie = raIdCookie.split("|");
		isAssociate = raCookie[5];
		craftsmanClubMember = raCookie[6];
		shipVantageMember = raCookie[7];
		sywrMember  = raCookie[8];
	} 
		
	if(sywrMember == "Y")
		memberStatus = 'SYWR';
	if(craftsmanClubMember == "Y")
		memberStatus = 'craftsmanclub';
	if(shipVantageMember == "Y")
		memberStatus = 'shipvantage';
	if(isAssociate == "Y")
		memberStatus = 'associateid';
	if((sywrMember == "Y")&&(craftsmanClubMember == "Y"))
		memberStatus = 'craftsmanclub|SYWR';
	if((shipVantageMember == 'Y')&&(sywrMember == "Y"))
		memberStatus = 'shipvantage|SYWR';
	if((shipVantageMember == 'Y')&&(craftsmanClubMember == "Y"))
		memberStatus = 'shipvantage|craftsmanclub';
	if((isAssociate == "Y")&& (sywrMember == "Y"))
		memberStatus = 'associateid|SYWR';
	if((shipVantageMember == 'Y')&& (isAssociate == "Y"))
		memberStatus = 'shipvantage|associateid';
	if((craftsmanClubMember == "Y") && (isAssociate == "Y"))
		memberStatus = 'craftsmanclub|associateid';
    if ((shipVantageMember == 'Y') && (craftsmanClubMember == "Y") && (isAssociate == 'Y'))
		memberStatus = 'shipvantage|craftsmanclub|associateid';
    if ((shipVantageMember == 'Y') && (craftsmanClubMember == "Y") && (sywrMember == "Y"))
		memberStatus = 'shipvantage|craftsmanclub|SYWR';
    if ((shipVantageMember == 'Y') && (isAssociate == "Y") && (sywrMember == "Y"))
		memberStatus = 'shipvantage|associateid|SYWR';
    if ((craftsmanClubMember == "Y") && (isAssociate == "Y") && (sywrMember == "Y"))
		memberStatus = 'craftsmanclub|associateid|SYWR';
    if ((shipVantageMember == 'Y') && (craftsmanClubMember == "Y") && (isAssociate == "Y") && (sywrMember == "Y"))
		memberStatus = 'shipvantage|craftsmanclub|associateid|SYWR';
//NOTE the ugly use of negating true conditions instead of using 'not equal'
    if (!(shipVantageMember == 'Y') && !(craftsmanClubMember == "Y") && !(isAssociate == "Y") && !(sywrMember == "Y"))
		memberStatus = 'NA';
    
    if(sywingo != null && sywingo != 'NA'){
    	if(memberStatus == 'NA'){
    		memberStatus = 'Roebucks';
    	}else{
    		memberStatus += '|Roebucks';
    	}
    }
    
	return memberStatus;
}

When it looks overly complex, it probably is! Repeatedly checking the same vars for the same values is a good indication that something is wrong. How many more lines will have to be cut & pasted when a new option – say ‘executive’ is added to the ra_id cookie as a membership type?
Look for ways to simply code and keep it DRY (Don’t Repeat Yourself). Eliminating single-use vars and repeated conditions cuts the number of lines in half and makes it easier to read and maintain.


function getMemberStatus() {
	var	raCookie = ($.cookie('ra_id') || '').split('|'),
		items = [],
		sywingo = $.readUpdateJsonCookie('c_i', 'sywingo');

	if (raCookie[8] == 'Y') {
		items.push('SYWR');
	}
	if (raCookie[6] == 'Y') {
		items.push('craftsmanclub');
	}
	if (raCookie[7] == 'Y') {
		items.push('shipvantage');
	}
	if (raCookie[5] == 'Y') {
		items.push('associateid');
	}
	if (sywingo && !!sywingo.sY && sywingo.sY != 'NA') {
		items.push('Roebucks');
	}

	return items.length ? items.join('|') : 'NA';
}

Appendix

Some additional reading


Performance by the pros

Crockford has already been mentioned many times. Resig should be required reading. Steve Souders is another guru. Try to get at least some of each in your thinking as you write code. Nicholas C Zakas, author of High Performance Javascript has been making a name for himself recently – here’s a slide show –http://www.slideshare.net/nzakas/writing-efficient-JavaScript that goes along withhttp://www.youtube.com/watch?v=mHtdZgou0qU&feature=related. Would recommend watching the video and tracking it as a training in your performance plan

Highlights:

Scope Chain

  • with and try-catch statements add another object to the scope chain – not good
  • Global vars are accessed after exhausting immediate scope chain, thus slower
  • Always use var, otherwise you’re making a global
  • for/do/while all process at about the same speed
  • beware conditionals, the more you evaluate, the longer it takes
  • combine control condition and control variable when possible
  • i > x takes longer than i– (as I eventually is 0
  • using each is much slower than a for loop

The Dom

  • live updates and HTML Collections
  • array iteration is much faster than collection iteration
  • watch Reflow
    • setting props on the fly takes more time. Every property touched requires reflow
    • better to just change a class when possible – then all properties are affected at once
    • consider removing an object from the DOM, making modifications, then adding it back

Another handy tutorial: http://www.webreference.com/programming/JavaScript/jkm3/

 

 

Post a New Comment

Your feedback will help us improve this page. If you have a comment about the content of this page, use the form below, and the Patterns Team will get back to you within a business day.

For general questions or assistance, email the Patterns Team directly.