What's wrong with your JavaScript Pt 1

As a senior level developer at my job, I end up having to do a lot of code reviews. I am also fairly active on Stack Overflow and their Code Review site. So this means I see a lot of JavaScript and jQuery. A lot. And what I see is a lot of the same mistakes made over and over again.

I don't know if it's insufficient training, a general lack of understanding of the language, or they are just used to writing in another language ( say C# ). So I wanted to write something up quickly to go over a few of these items.

First, let me state this for the record. I do not consider myself a great developer. I know how much I don't know. I am still learning every day. And that's the way it should be.

With that being said, I am a damn good developer. I can easily spot a lot of common code smells. Addy Osmani has one of my all time favorite quotes about development ( emphisis mine ):

First do it, then do it right, then do it better.
    -Addy Osmani

Scripts at the bottom

To this day, I still see people including their script tags in the head of the HTML document. Stop! Due to the way the browser downloads a web page, it is a best practice to put these right before the closing </body> tag.

The reason being that the browser must parse it, and execute it before it can continue downloading the page. Since this affects how fast the user can see the page, you want this to occur after all the HTML has been downloaded.

Also, if your JavaScript gets executed before the DOM finishes loading, and you are referencing an element in your code, it will fail.

FYI: For CSS, you should keep them in the <head> since it controls how the HTML will look after its downloaded.

Create a Private Scope Using an IIFE

Originally coined by Ben Alman back in 2010, IIFE stands for Immediately Invoked Function Expression. If you aren't using an IIFE in your code then you are doing it wrong. I am not going to go into great detail here. If you don't understand scope and closure, there are a few places to find out more.

Here is the gist: Using an IIFE creates a closure which gives you private scope for your code. This will help you avoid conflicts with other peoples code. It also helps keep your code out of the global scope. And we all ( should ) know, having a lot of code in the global scope is pure evil.

So before you sit down to start writing your code, the first thing you should do is create an IIFE:

(function( $, window, document, undefined ) {
  // code goes here
})( jQuery, window, document );

In this specific example, I am passing in jQuery as the $ sign. This way, in all of my code, I know that $ is jQuery and not another library or function. If you don't need jQuery, you can remove it. I also pass in the window and document objects. This will allow me to refer to them locally which provides a slight performance boost.

Notice the last parameter. It is received at the top of the code but is not sent at the bottom of the code. This prevents undefined from being re-defined by the user in older browsers. As of ES5, this can no longer be changed. So we are talking IE8-. Most other browsers that don't support ES5 have long since gone the way of the dinosaur.

Use Namespaces and Objects

This item ties back in with the item above. If you keep all of your code out of the global scope and in an IIFE, how do you access it later? Well the easiest way is to create a single JavaScript object in the global scope and add all of your code into this one object. This will effectively create a namespace for you.

So say you are creating a shiny new carousel control (why? We have 1000s already! ). It's called 'FinalCarousel' because you know it's the last one anyone will ever need. So, let's create our IIFE and set up our namespace:

(function( $, window, document, undefined) {

  function go(num) {
    // function to change slide

  function handleEvents() {
    // function that handles JavaScript events
    // calls go to increment or decrement slides

  function initialize() {
    // starts the courasel 
    // calls handleEvents

  function dieCarouselDie() {
    // remove courasel

  //attach to the global scope
  window.finalCarousel = {
    init : initialize,
    destroy : dieCouraselDie

  //after the DOM is ready call our init function
    // finalCarousel.destroy();

})( jQuery, window, document );

Now, we have attached our namespace finalCarousel to the global scope by adding it directly to the window object. All of our code is encapsulated inside of our namespace. There are a couple of other things of interest here as well.

First, notice the difference between the internal name and the external name for the functions. Our function is actually called dieCarouselDie which is, admittedly, a bad name. But the point is you can make your internal names as descriptive as you want them to be. The external name is what is actually used and, since it's attached to our object, can be a much simpler name.

Second, notice that the handleEvents and the go function are not available externally. We didn't add them to the object so the are not exposed to the user. But, because they are part of the same scope, they are available to be called by the other functions that are exposed to the user. In other words, they are effectively private methods.

Always "use strict"

Lets take the above code. It's missing one very valuable line of code. And that is the 'use strict';. Putting this inside of your code makes the JavaScript parser run in ( you guessed it ) strict mode. You want this; trust me. If you want to know more, check out this article by John Resig. The easiest way to implement this is to just include it immediately inside your IIFE.

(function( $, window, document, undefined ) {
  'use strict';
  // code goes here
})( jQuery, window, document );

Important Note: You should never add use strict to the global scope. Always add it at a function level.

Use = instead of

I see this a lot especially with developers coming from other languages like C++ or C#. In those languages == functions the same as === does in JavaScript. So what is the difference between the two in JavaScript? The double equal checks just for equality. For instance:

false == 0  // returns true

This will return true because both false and zero are false-y values in JavaScript. However this:

false === 0  // returns false

The triple equals check for equality and type. That is an important distinction. false is a boolean and 0 is a number. These are different types that have the same value. In JavaScript, unless you have a specific reason not to, always use the triple equal.

Stop Re-evaluating the Same Thing

A great example of re-evaluating the same thing over and over is this code for adding cross-browser events:

function addEvent( event, target, method ) {
  if ( target.addEventListener ) {
    target.addEventListener( event, method, false );
  } else if ( target.attachEvent ) {
    target.attachEvent( 'on' + event, method );

Every time this function is called it checks to see which type of event method the browser handles. Why? The browser isn't going to change! Let's fix this problem:

var addEvent = ( function() {
  if ( window.addEventListener ) {
    return function(event, target, method) {
      target.addEventListener(event, method, false);
  } else if (window.attachEvent) {
    return function(event, target, method) {
      target.attachEvent("on" + event, method);

In the above code, we have an IIFE that returns a function expression to the variable addEvent. When this code is executed, it figures out which event method to use and returns that function. So, every time addEvent is called, it is only executing the correct function based on browser support. No more checking every time the function is called!

Optimize Your Loops

How many times do you see this code:

var myArray = [ 1,2,3,4,5,6,7,8,9,0 ];
for( var i = 0; i < myArray.length; i++ ) {
  // do whatever here

So what's wrong with that? Well, each time through the loop, JavaScript is re-evaluationg myArray.length. Why force it to do extra work? We already know what the length is and in this case, it's not going to change. So much like your variable i, let's set it up before hand.

var myArray = [ 1,2,3,4,5,6,7,8,9,0 ];
for( var i = 0, len = myArray.length; i < len; i++ ) {
  // do whatever here

This way, len is evaluated at the start of the loop and is used instead of the myArray.length look up. If you are looking for that little bit of extra speed and the order of execution isn't important, you can do this even faster with a while loop:

var len = myArray.length;
while( len-- ) {
  //do whatever here

So why is this faster? A couple reasons. In the for loop, we have to take value of i and compare it to the value of len before we can do anything. With the while loop, we simply decrement len and execute the code. It will automatically complete the loop once we reach the end of the array ( zero is a false-y value ). You can check out this jspref to see it in action.


I think that is enought to get started. I will do a follow up that including: DRY-ing your code, caching selections, throttle/debouncing events, and a few more jQuery specific items.

'Til next time!