Wednesday, August 8, 2012

Fixing a broken codebase, part I

There's an interesting post in Ars Technica by a software engineer who has been hired by a group of scientists to modernize their development practices and fix a big mess of spaghetti-code that has accumulated over 10-20 years. He wants a few fairly simple techniques that will make a big difference.


In my opinion, the engineer should start by instituting some basic modern development practices. They won't fix the broken codebase, but they will keep things from getting worse. They'll also lay a foundation of good practice that will make later improvements possible.

The first phase should institute:
  1. Source control. The scientists need to be able to control who does what to the codebase, and keep a history of changes. A modern source code control system like Perforce or Mercurial does exactly that.
  2. Code review. Every change to the code should be examined by someone other than the original author, and that examiner should be empowered to demand changes until he is satisfied the new code is sound. Many eyes make all bugs shallow, and it starts with code review.
  3. Code ownership. The system is too large for anyone to understand well. It should be divided into portions, based on who understands what best about the system as it is now. Once that's done, the code review policy should mandate that all reviews include the owner of the code being changed.
  4. Daily builds. It should be possible to check out the complete codebase from the source code control system and build (compile and test) it from scratch. This should be done at least daily. It may be best to assign staff to a rotation, with each person being build engineer for a day. In addition to building the current codebase, the build engineer is also responsible for fixing the build if it is broken by identifying the offending code and rolling it back. The build engineer is done when the codebase builds cleanly again.
  5. Bug database. All error reports and feature requests should be stored in a central searchable repository, so the code owners know what is broken and what has been fixed.
  6. Design documents. All new functionality should be documented up front, presented for peer review, and revised until approved. The documents don't need to be very detailed, but they should describe the essential elements of the proposed functionality and why it is being introduced. Also, if the designs change during development, the documents should be updated to reflect what was actually built. And the design documents should be kept in some readily accessible place.
  7. Unit tests. All new functionality should include tests that verify that the code performs as expected. Going forward, these tests will make sure that any additional changes do not break existing functionality. These tests should be run as part of every daily build. Also, all fixed bugs should have unit tests that reproduce the failure, but which run correctly with the fix in place.
These seven policies will keep things from getting worse. If the team did nothing else, they would have their old mess of a codebase, plus a slowly growing layer of superior new code that accesses it. Over time, the average quality of the codebase would slowly rise, as more and more code would be of the new type.

In part II, I'll describe what can be done to improve the older code.

No comments:

Post a Comment