Code Renaissance is about building great teams and great software. By exploring best practices, team interactions, design, testing and related skills Code Renaissance strives to help you create the team and codebase that you've always wanted.

Cleaning up Bad Javascript with JSLint

The web app I'm working has a ton of Java Script. It was written by many people, over a short period of time, under incredible deadlines. To make matters worse the direction of the product changed several times during initial development, which required things to be shifted and duck taped together quickly with no time for cleanup. Needless to say, the code base is a mess.


So what's the answer to cleaning up a brittle Java Script code base?

JS Lint!

JSLint is life, JS Lint is sanity. Yes it can be a huge pain and even it's author Douglas Crockford notes "JS Lint will hurt your feelings", but JSLint is the first step in solidifying the most horrid code base.

When I run a file through JS Lint for the first time I inevitably find several problems:
  • Unreferenced code - it's very difficult to make sense of code that has unused functions and variables; how can you know what's relevant? JS Lint will flag these for you so you can remove them.
  • Bad practices - While I'm sure JS Lint's list of bad practices will include things you disagree with, it's worth it to fix all of the minor errors so that the truly horrific things will be revealed. Be very careful with your changes though. When JS Lint says you should use !== instead of != for comparisons doesn't mean that's the exact change you should make.
    if(a != 0) could need to be changed to any of the following:
          if(!a)//excludes any falsy values (NaN, '', false, 0, null, undefined)
          if(a !== 0)//excludes 0
          if(parseFloat(a,10) !== 0)//excludes 0 and '0', but not '' or 'abc'
          if(parseInt(a,10) !== 0)//excludes 0, '0', but also 0.13, '.13'( truncated .)
    
  • Undeclared global references - JS Lint flags undeclared global references. Some of these will be objects that are missing a declaration, some will be mispelled variable names, and some will be intentional global references to other js files. Once you comment your intentional globals JS Lint will no longer flag them. I prefer to comment my globals like this:
    /*globals
    Authorization, ApprovalTypes, //Authorization.js        
    jQuery, $, //jQuery 
    window //browser
    */
    

Once you've gotten your file to pass JS Lint and tested your changes then you need to do a check-in. This gives you a stable starting point to begin refactoring the code. I'll admit this can be dangerous but rerunning the code through JS Lint every so often as you're making changes catches a lot of issues. Everytime you revisit a file to make a change refactor it a little more. If the function you're working on is too big and too confusing then extract out its different responsibilities into cohesive methods. Clean up the code until it makes sense and you can reliably make your change.

It is critical to resist the temptation to clean up code that will not be tested. Don't fool yourself into thinking you won't introduce bugs, you will. That's why it's equally critical to take advantage of the opportunity to clean up the code while you're making functional changes. This will allow your clean up efforts to be tested. Don't Pass up an opportunity to clean up code while you're in it... that's how the code became brittle in the first place.

New Team Members and the Five Stages of Grief

After years of watching new developers join teams (and from time to time being one myself) it occurred to me that the five stages of grief apply to this transition as well.

  1. Denial – "This code makes no sense… I must be missing something."
  2. Anger – "I can’t believe this $#!%. What kind of moron designed this?"
  3. Bargaining – "I realize the code works but it’s brittle, confusing and impossible to maintain, can’t we refactor it a little at a time? ... What do you mean it’s too critical?"
  4. Depression – "The code base is crap and they won’t let me fix it… I might as well shoot myself now."
  5. Acceptance - “Hi you must be the new guy… Yeah, I’ve been here a year now… Oh that code, yeah it's not ideal but it works… No we can’t change it, it’s too critical and there's no time... Hey, there’s no point in getting upset about it, it’s just a job.”