It's one thing to write code from scratch, but quite another to edit and improve existing code. It's also quite something to have your code dissected in front of a crowd...

Refactoring

In its purest form, refactoring is changing code without changing its functionality - work done purely to make the code faster, more robust or maintainable.

Most of the time work described as "refactoring" ends up being more of a rewrite, as few people can resist the opportunity to change functionality. Rewriting can be a time blowout though, so the strict scope of pure refactoring can be useful.

If it ain't broke...

A fair question is of course if it's not broken, why fix it? Why change code that works?

Yes, if the code in question is fast, bug-free and easy to update then you may be better off leaving it alone. But in the real world very little code falls into this category on the first version, particularly if it's written by someone who doesn't expect to maintain it (the joy of outsourcing...).

No matter who writes it, most code is written to a deadline. The compromises made to ship it on time usually come back to bite you in maintance - you never get it for free, you just choose when to pay.

Even if you wrote it and you weren't under a deadline, the second version of your code may still be better than the first. Why? Because you learned about the task when you wrote it the first time, so you rewrite it armed with specific experience.

This is one reason iteration works; and one reason a quick spike or prototype can be a great investment even when you throw the code away.

Refactoring is also an effective way to share knowledge, particularly if you pair with someone more experienced while you do it. The goals are already defined and understood - you can see how it works, so you'll know when the new version does the right thing.

Which leads me to Refactor Wars...

Sydjs refactor wars

At the May Sydjs I took the daunting step of having four people refactor one of my scripts. As a recent convert to Javascript, my code is definitely newbie code. So why have it dissected in front of the JS Ninja-laden Sydjs crowd?

It goes back to a classic problem of user goups: how do you share knowledge from the ninja to the n00b? If you just show newbie code, people get bored. If you just show ninja code, the newbies aren't really progressing.

Refactoring is one way to bridge the gap, so I organised overhauls of Is it laksa time yet? - a simple script which shows different messages as the user's system time approaches Friday noon (aka laksa time!). I wrote it quickly as a bit of fun for the Sydney web laksa crew.

You can see my intro slides and the four refactored scripts at Is It Refactor Wars Time Yet?. Since then I've done some basic analysis of the presented code.

Head to head

The original script used functional but flawed jQuery. The following approaches were used to refactor it:

  • Improve the jQuery
  • Replace jQuery with Zepto (a bit of an experiment since Zepto's aimed at mobile webkit rather rather than the desktop)
  • Remove jQuery (ie. convert to straight Javascript)
  • Write a reusable library (without jQuery)
  • Rewrite in Coffeescript
  • Although it wasn't presented on the night I've included my own update, which removed jQuery and stripped out some ultimately pointless abstraction.

Remember to take these numbers with a grain of salt, as the tests and measurements were not taken in a hermetically sealed laboratory by trained professionals - I just did them on my home PC. They simply give a rough idea of the results.

  Original Lindsay Chris (Zepto) Chris (no jQuery) Steve Gabe Mine
Summary jQuery Better jQuery Replaced jQuery with Zepto Removed jQuery Removed jQuery, wrote library Rewrote in Coffeescript Removed jQuery
Requires jQuery 1.4 jQuery 1.6 Zepto none none jQuery 1.4, Coffeescript none
LOC 67 56 67 50 108

CS: 64
JS: 71

 40
Page weight (kb) 34.2 38.3 8.2 2.8 4.2 32.7 7
Execution time (ms) 0.041932 0.038928 0.100244 0.005836 0.034735 0.027839 0.006083
Execution +/- (%) 0 -7% +139% -86% -17% -33% -85%
Notes Written quickly. Check out the mad ternary version! Breaks in Firefox (uses innerText)   Steve says it needs a refactor. With tests: LOC 154/141. Exec time 0.157247 (+275%)
 2.3kb without the ascii art!

Notes for the curious:

  • Execution time is the average of 1,000,000 repetitions in Chrome.
  • Page weight is non-gzipped (it's not enabled on the laksa site), plus the gzipped weight of jQuery and Zepto (since jQuery is loaded from a CDN).
  • For Gabe's option, the weight and execution time used the generated javascript. For execution time, tests were disabled.
  • For Gabe and Steve's options, I had to add a wrapper function to enable testing. So in production execution would be faster/cheaper by one function.

Conclusions

What do we learn from these numbers and the refactoring exercise in general?

First up, several people observed that in a production scenario they probably wouldn't bother refactoring the script. It only executes every ten seconds and even the slowest version takes less than one millisecond.

Yes, it was definitely flawed; but it didn't really matter... and just to highlight the fact it's not risk-free to change things, when I refactored it myself I introduced a bug!

However when viewing this as a learning exercise, it's worth noting a few things:

  • Every option except Zepto improved the performance of the script.
  • Improving the jQuery option's performance by 7% could be quite signficant in a real world scenario.
  • While jQuery's weight looks bad on paper, it can load off the CDN faster than the base page.
  • Simple things like caching selectors can make a huge difference, whether you're using a library or not.
  • If you don't need jQuery, don't use it. For a page this small, all it was doing was element selection and $(document).ready - easily replaced with document.getElementById and moving the script to the end of the document.
  • Don't guess about performance, test it. Even my dodgy little profiling script gives far better information than guessing.
  • Lines Of Code can be a very misleading metric, unless you see an extreme change it probably isn't worth worrying about. While readability is subjective and hard to evaluate, LOC is not a substitute.
  • Remember most things are a trade off:
    • jQuery is fast and easy to write, but adds weight and is slower than straight Javascript.
    • Writing a library creates more lines of code but changes a one-off script into something reusable.
    • Porting to Coffeescript adds a significant dependency but would make sense if your team is more productive using it.
  • Ascii art adds a surprising amount of page weight. But it's fun.

Refactoring for fun and profit

The best thing about presenting Refactor Wars at Sydjs was showing some very different approaches to the same scenario.

The clearest result was that no single approach will always be right - each one would be the most appropriate option in a different situation. You need to understand your current task's priorities and work to them. But, remember you're always making a tradeoff; for example if you ship fast, you'll almost certainly incur a higher maintenance cost.

The exercise probably also made the laksa script one of the most heavily-refactored joke sites in history, not to mention taking out the record for the largest amount of actual code shown in a single Sydjs! Hopefully everyone got something out of it.

(That link again, if you want to see the scripts see Is it refactor wars time yet?)