Tips for cleaning up spaghetti JavaScript

When people start to learn JavaScript, they have a tendency to write all their code inside one giant $(document).ready() function. One big, tangled, untestable, hard-to-debug block of code that does everything: event handling, AJAX calls, DOM manipulation... We've all been there.

This approach might work for a bit, but as you start to write bigger programs, or if you (or someone else) tries to extend your code, numerous problems will start to slow you down. One of my most valuable (and satisfying) breakthroughs in learning to code was starting to think about how to structure my programs, and discovering there were design patterns I could use to make my code more robust.

Here are a few techniques you can use to refactor jumbled code, or start to move away from writing it in the first place.

The module pattern

I generally use this if I'm writing a small enough amount of JavaScript that it's not worth using a framework (under 200 lines or so). Examples might be a quick front-end prototype, a standalone promotional or status page, or a static portfolio site. Say we have a navigation component that fetches the nav items in JSON format, sorts them, and creates the nav. Here's what our code might look like using the module pattern:

$(document).ready(function() {
  Nav.run();
});

var Nav = (function() {
  // Private functions go here.
  function sortNavItems(items) {
    return items.sort(function(a, b) {
      return a.name > b.name;
    })
  }

  // Methods to be exposed in
  // your module's API go here.
  return {
    run: function() {
      this.getNavItems(this.createNav);
      $('#nav').on('click', this.handleClick);
    },
    getNavItems: function(callback) {
      $.getJSON('/nav-items', callback);
    },
    createNav: function(items) {
      var sortedItems = sortNavItems(items);
      // More code
    },
    handleClick: function(event) {
      // More code
    }
  };
})();

By treating the navigation as a module, we can scope its functionality using an IIFE, make that functionality available through the Nav variable, and choose which of its methods we want to expose, either for other parts of the program to interact with, or for testing. It's also important to try to split up functionality so that each function only does one thing. We can repeat this process for other discrete components, widgets or sections of our page, encapsulating each of them in their own module.

The constructor pattern

This is an alternative to the module pattern described above, which can be a slightly better choice if you have repeated similar components on your page (e.g. multiple menus or search boxes), as it allows you to create multiple object instances from the same constructor. Again, it's probably most useful in projects without a framework as most frameworks come with their own ways to do this (Backbone views, Angular controllers or directives, React components etc.).

We can re-write our nav example using the constructor pattern like this:

$(document).ready(function() {
  var nav = new Nav('#nav');
  nav.init();
});

function Nav(el) {
  this.el = $(el);
  this.el.on('click', this.handleClick);
}

function sortNavItems(items) {
  return items.sort(function(a, b) {
    return a.name > b.name;
  })
}

// Store methods to be exposed on the
// object's prototype property.
Nav.prototype.init = function() {
  this.getNavItems(this.createNav);
}

Nav.prototype.getNavItems = function(callback) {
  $.getJSON('/nav-items', callback);
}

Nav.prototype.createNav = function(items) {
  var sortedItems = sortNavItems(items);
  // More code
}

Nav.prototype.handleClick = function(event) {
  // More code
}

Here, we're setting any event handlers in the constructor function, and defining methods on the prototype. This means they can now be inherited by other instances of the object. We could also define them inside the constructor function (this.init = function(){} etc.), but this would mean redefining each function for every instance of the object we create. This would be less efficient, and also mean we couldn't benefit from prototypal inheritance.

The prototype pattern is another way of achieving this kind of functionality with objects.

A global event bus

Another thing that tends to happen with framework-less JavaScript is tight coupling through events. Take this example:

$('#signup-button').on('click', function(event) {
  $('#signup-form-popup').show();
});

There's nothing technically incorrect about this code, but it's not good design. The problem is that the button and popup are actually separate parts of the UI. They shouldn't have to know about each other.

Tightly coupling elements like this makes debugging hard. Say I click the button and the popup doesn't show, how do I know if the problem is with the button or the popup? We have to take a random guess on where to start looking. Obviously this is a trivial example, but if you have a 50-line function handling one long process and no easy way to tell where the bug might be, things get confusing.

Rather than have separate parts of your UI depend on each other like this, we can broadcast events globally from one part of the UI, and listen for them in others. With jQuery, you can broadcast a custom event with:

$(document).trigger('eventName', param1, param2);

And then listen for it with:

function doSomething(event, param1, param2) {
  // Do things
}

$(document).on('eventName', doSomething);

By always using the $(document) object, if you change or remove an element further down the DOM tree, there's less chance of your code breaking due to old references that you forgot to update.

Frameworks use similar techniques for implementing this, for example $scope.$broadcast(), $scope.$emit() and $scope.$on() in Angular. Marionette.EventAggregator is a nice option for Backbone.

Dispatch tables

Consider this function:

function handleMenu(menuItem) {
  switch (menuItem) {
    case 'item1':
      // Do things
      break;
    case 'item2':
      // Do things
      break;
    case 'item3':
      // Do things
      break;
    case 'item4':
      // Do things
      break;
    case 'item5':
      // Do things
      break;
    case 'item6':
      // Do things
      break;
    case 'item7':
      // Do things
      break;
    case 'item8':
      // Do things
      break;
    default:
      // Do a default thing
      break;
  }
}

Switch statements can be fine for 3 or 4 conditions, but any longer than that, and you could consider refactoring them into a dispatch table:

var menuItemActions = {
  item1: function() {
    // Do things
  },
  item2: function() {
    // Do things
  },
  item3: function() {
    // Do things
  },
  item4: function() {
    // Do things
  },
  item5: function() {
    // Do things
  },
  item6: function() {
    // Do things
  },
  item7: function() {
    // Do things
  },
  item8: function() {
    // Do things
  },
  default: function () {
    // Do a default thing
  }
};

function handleMenu(item) {
  var action;

  if (menuItemActions.hasOwnProperty(item)) {
    action = menuItemActions[item];
  } else {
    action = menuItemActions.default;
  }

  return action();
}

By storing all your actions in a menuItemActions object, you shorten your handleMenu function and make it much cleaner and easier to understand. Your code is also more extensible - if you need to add an action, simply add another function to the actions object. You don't have to worry about adding another case: or break; to an unwieldy switch statement. But perhaps the biggest benefit of this approach is that because your actions are now functions, each one is individually testable.

This technique is very similar to the command pattern.

These are just a few suggestions for how you can start to write more readable, more testable, easier-to-debug JavaScript. Some more general tips are keep your functions small, ideally handling a single task each. Also, write tests! Effective tests make debugging so much easier, so it's a skill really worth learning. Learning JavaScript Design Patterns is an awesome book if you want to learn more about ways to structure your code, and Writing Testable JavaScript is a really good article if you're struggling to get started with testing due to not yet managing to write testable code.