prettyprint

Wednesday 30 January 2013

How to write maintainable javascript (or die trying) part I

I think I finally got it! After years of doing experiments I finally found the secret formula for maintainable javascript. I would like to share it, as well as invite you to peer review it.

(Edit from 2014 Meanwhile my coding style dramatically evolved from this, I'm keeping this post here more as a legacy reference)

When I began working with JavaScript The first thing I did was learning jQuery and watch the David Crawford's video Javascript the Good Parts. Afterwards I started a project which was a Ganttchart web app. If you dare to inspect it you will see the most messy JavaScript ever written. Meanwhile I kept working on doing other front end stuff, never dropping jQuery but always trying to improve my code style.

I don't want to bug you with the full story so I'll just cut to the chase. Following are the issues that most commonly occur when developing front end stuff: (This article addresses some of them)
- Declaring lots of loose functions/variables in the same scope.
- Repeatedly querying the DOM for the same thing
- Overuse of JavaScript closures
- Cross dependencies everywhere
- Using the DOM as a place holder to store variables and states
- Doing mega selector chains in jQuery

Each one of these issues has a reason to exist, otherwise they would not happen. Remarkably when you find code that has one, you will easily find signs of the others. Typically declaring lots of loose functions is the first code smell. You can argue that you have them only doing a stateless thing in the interface like:
function selectFriends(firstName){ 
    $('div.friend').each(function(i,e){
        var names = $(e).attr('data-name').split(); 
        $(e).toggleClass(‘selected’, names[0]==firstName); 
    }); 
}

function removeFriends(firstName){ 
    $('div.friend').each(function(i,e){ 
        var names = $(e).attr('data-name').split(); 
        if(names[0]==firstName){ 
           $(e).remove(); 
        } 
    }); 
}

If you only have the first function, I will not complain that you should encapsulate it. But the moment you start adding more functionality things will get messy.

Lets Identify the main issues present in that code:
- Every time we want to select or remove friends we are querying the DOM
- Both functions are dependent on the string “div.friend”, so if you want to change this you would have to edit it everywhere. Imagine if you have many references… on different files!
- Duplicate code.

To address these issues, it is always a good idea to use a map, after all JavaScript is all about maps. So why not run some code before these functions are called, which should build a simple data structure to store our references? As we are only doing operations on the first name we can use those as keys. Afterwards we can then store a reference to the DOM as the paired value. As names are not unique lets use a list of DOM references, so we can store multiple ones.

With this in mind we can now refractor the above code into:
var friendNames = {};

$('div.friend').each(function(i,e){ 
    var name = $(e).attr('data-name').split(); 
    if (!friendNames[name[0]]){ 
        friendNames[name[0]] = [$(e)]; 
    } else { 
        friendNames[name[0]].push($(e)); 
    } 
});

function selectFriends(firstName){ 
    for(var i=0; i < friendNames[firstName].length; i++){ 
        friendNames[firstName][i].addClass('selected'); 
    } 
}

function removeFriends(firstName){ 
    for(var i=0; i < friendNames[firstName].length; i++){ 
        friendNames[firstName][i].remove(); 
    } 
    friendNames[firstName] = []; 
}

In this new code we are no longer querying the DOM every time we want to select or remove a friend. The logic behind each function is also much clearer.

But a new issue arises: we are still writing all the code in the global scope, which isn't nice. So we should encapsulate it into an object which explicitly dictates its functionality. I like to use singletons for interface controllers that specifically handle a single functionality in the interface. To do that I store a reference to itself in its prototype.

So our object would look like this:
function FriendHandler(){ 
    if(FriendHandler.prototype.singleton){ 
        return FriendHandler.prototype.singleton; 
    }

    if(!(this instanceOf FriendHandler)){
        return new FriendHandler();
    }

    FriendHandler.prototype.singleton = this;

    var NODES       = { friends : $('div.friends') },
        friendNames = {};

    NODES.friends.each(function(i,e){ 
        var name = $(e).attr('data-name').split(); 
        if (!friendNames[name[0]]){ 
            friendNames[name[0]] = [$(e)]; 
        } else { 
            friendNames[name[0]].push($(e)); 
        } 
    });

    this.selectFriends = function(firstName){ 
        for(var i=0; i < friendNames[firstName].length; i++){ 
            friendNames[firstName][i].addClass('selected'); 
        } 
    };

    this.removeFriends = function(firstName){ 
        for(var i=0; i < friendNames[firstName].length; i++){ 
            friendNames[firstName][i].remove(); 
        } 
        friendNames[firstName] = []; 
    };

    return this;
}

We now have a nice object with public functions which anyone can easily understand. Any coder who reads FriendHandler().selectFriends(‘Jonh’); will easily follow the logic flow and quickly understand what is happening. On a side note a friend of mine, to whom I requested a review of this article, made this cool jsFiddle with an example of the code above.





Some details:

In this last code there are still some issues, one of them is: what happens if new html shows up? It's very common to have an ajax call appending new friends. Then our object would contain an inconsistent representation of the DOM, in both the NODES.friends and in the friendNames map.

To solve the first problem, which consists in the jQuery object representing old HTML entries. This representation could contain elements which are no longer present in the DOM and miss new ones which where added later. I created a small jQuery plugin, so small that it fits in a gist on github (link), to allow easy updating of jQuery elements stored in variables. With this plugin we just need to call update() to have the an updated version of the DOM without having to run the selector again.
    NODES.friends.update();
Know that we know of a way to update the jQuery object which we have stored, it is only a matter of creating a function to update our map. We can this way add the following to our object:
function _update(){
    friendNames = {};

    NODES.friends.update().each(function(i,e){ 
        var name = $(e).attr('data-name').split(); 
        if (!friendNames[name[0]]){ 
            friendNames[name[0]] = [$(e)]; 
        } else { 
            friendNames[name[0]].push($(e)); 
        } 
    });
}

this.update = function(){
  _update();
};


I hope you have found this article both useful and enjoyable. I will eventually write part II where I want expose how to write event subscription systems based on Observers. Thank you for reading and if you have any questions or feed back please leave a comment and I'll be glad to reply.

7 comments:

  1. Interesting post! Keep posting

    ReplyDelete
  2. Wouldn't using a module system (e.g., RequireJS) and a MV* framework (e.g., Backbone) solve most of these problems?

    ReplyDelete
    Replies
    1. Great question. The TL;DR; answer is: Yes!
      I particularly like knockout and believe it solves lots of modularity issues. But my point with this article is maintainability. Althought using these frameworks (backbone, knockout, amber, ...) provide you with a shortcut to solve multiple issues, they also enforce an extended learning curve to any engineer who is going to extend/mantain your code. There are two main points that make me be against these frameworks: 1- You cannot expect to have expert developers maintaining your code. Often trainees and developers from other fields have to extend/fix issues in your code. 2 - These frameworks are hard to add to existing projects, where there already exists a big code base to maintain. You will be adding a new code paradigm/structure to your existing project.
      Which means that if you can develop modular and maintainable code, you won't need to add another layer of complexity to your code. Currently I want code to be simple to any developer, independently of which field he is comming from, and with minimal learning curves so they can be toping productivity all the time.

      Delete
    2. Regarding existing projects, I mostly agree, although it is not fair, IMHO, to put Backbone, Ember and and Knockout in the same integration difficulty bag. Backbone is way easier to integrate into existing projects that Knockout because it is much less opinionated (Ember being somewhere in the middle).

      In terms of learning curve I agree that can be a problem. But again, and to give an example, Backbone abstractions are much more simple and easy to understand than Ember's because they are much closer to what your usual jQuery user is accustomed to: binding to DOM events, creating new nodes and appending them to the live DOM, making AJAX calls, etc etc. On the other hand Ember has a custom rendering pipeline and framework managed DOM events. So my point with this is: I don't think it is correct to talk about these frameworks as if they were all (or mostly) the same (although you could say that about some others).

      Now, I'm not saying that we should always use frameworks. If, like you said, It's necessary for outside people to make changes to the code base, then the less abstractions you have the easier it will be for them to make the required changes. The problem with that, for me, is that it does not scale for large (client side) projects. If you start to have thousands and thousands of LOC you will need some abstractions (to keep things DRY), and I think it's a safer and better approach to leverage very mature and stable ones (like Backbone) that to implement in house because they will almost certainly be worse that something that was developed and used by so many people.

      I guess my point is: for a big projects you need abstractions and you might as well use what the community was to offer instead of reinventing the wheel. That might put some barriers to entry, but on the long run it should pay off. For small projects, I think what libs you use (or not) is something to evaluate individually.

      Delete
    3. Agreed, but my point with this article is JavaScript, not its frameworks. You still need to be able to write proper code without having to depend on external stuff. Thanks for your thoughts though.

      Delete
  3. Problem with using native Javascript structuring facilities is that only people fluent is JS will be able to understand them. Moreover they must be willing to do so...

    It's nice seeing this worked out but I think it's still to complicated to work with others on it.

    ReplyDelete