Monday, February 11, 2008

Anatomy of a Refactoring

The first part of bug 413260, refactoring nsIAbCard, is finally starting the review process, freeing my up to start on part two, nsIAbMDBDirectory. The goal here is to remove this heavily-used interface. For those of you who are only being introduced to large-scale refactorings, here is a simple step-by-step guide for refactoring.

  1. Pray that only a little JavaScript is involved. As much as people fall in love with JavaScript, I greatly prefer C++. Cases like this prove why: C++ complains when you compile that something goes wrong; these simple problems are deferred until actual execution in JavaScript. Sometimes, these problems crop out in the most out-of-reach places: one usage of nsIAbCard, unfound by grep, exists in msgHdrViewOverlay.js, one of the last places one would expect to find address book usages.
  2. Fire up grep and find usage characteristics. In the case of nsIAbMDBDirectory, I see that it is used outside of addrbook for two reasons: cardForEmail and to get the database. The former is simple to deal with via a minor refactoring, the latter requires some more in-depth analysis.
  3. Mark stuff as deprecated and compile. Note that gcc 4.3 or better is required to catch the most common case (nsCOMPtr stuff) and that, as of right now, my XPIDL/nscore.h patch is needed to mark IDL files as being deprecated. If you're using gcc 4.3, -Wno-conversion is highly recommended.
  4. Ensure that the tests use the new stuff. Tests are the simplest JavaScript to handle, primarily because everything is used. They also alert you to broken migration.
  5. Remove deprecated and change other JS. Don't forget to test the crap out of it. This will by far take the longest to execute. Stuff can crop up in weird places; JavaScript analysis is on my list of things to do, but I'm looking at IDL/C++/JS+Mozilla+ctags+vim automagic first.

1 comment:

zooplah said...

I have to agree with you on point 1. Statically-typed languages are so much less hassle in the long run. The LISP programmers always make a deal about how Pascal (my favorite of the programming languages I've tried) is a "bondage and discipline" language, but I've found that it helps make your programs work well when the compiler throws a fit, rather than when your program in a dynamically-typed language just quits or does something wrong and you don't know what the cause was.