As a part of my day-job, I was tasked earlier this year with improving the stability of our software stack. This was a wide ranging brief, involving tracking down and fixing bugs at every level, from the kernel up to the UI. It often required poking into the dusty corners that other programmers preferred to ignore, unearthing weird behaviours we never had the time to understand during development. On the whole, it was a task which demanded a cast-iron cynicism, a well-developed sense of suspicion, and not a little patience!
Over the course of my time on stability-watch, I noticed that I was increasingly guiding my suspicion by a barely-conscious mental checklist, a list of signs that a particular area of the code might be worth a further look. Like Joel On Software’s 12-point team checklist I was developing my own secretdogproject checklist of code quality.
So, with no further ado, here’s my checklist of bad signs to look out for in code. These aren’t language-specific issues, since most coders are pretty good at picking these out. Rather, they’re more holistic signs of quality in a code base which might help signpost areas of poor quality for review.
Does the code live in a sensible place and have a sensible name?
Each component of a codebase should have a name that communicates something about what that component does.
When combined, components should make use of a coherent directory hierarchy in order to communicate something about how the components fit together as a whole.
Does the code have any documentation?
As any true hacker knows, the best approach when in doubt about what some code does is to use the source. But sometimes, a paragraph of top-level design information, written in a high-level human language like English, can save hours of grubbing around in the code.
At a bare minimum, you should have a readme. Bonus points for autogenerated API docs.
Does the code build cleanly?
Compiler warnings are a hint that your code is wrong, and you ignore such hints at your peril. Good code should compile without warnings.
Bonus points if you’ve configured the build to treat warnings as errors to prevent future warnings creeping into the build.
Does the code have a test strategy?
Code can be tested in many different ways. Some components lend themselves to unit testing. Others are better tested in-situ using test harnesses or other scaffolding. Whatever the best approach is for your component, you should have a developer test strategy in place, and it should be obvious how to use it.
Bonus points for including links to archived test results in the component documentation.
Does the code look nice?
If people spend time making a thing look nice, that probably means that they care about that thing. This applies as much to code as it does to a house or a garden. A consistent aesthetic in code means that the implementers took the time to make it look good, which may imply that they also took the time to consider corner cases, double check API call documentation, and prototype the implementation to chase out any unforeseen snags.
It doesn’t matter, by the way, what your coding convention is — so long as one is evident, and it is consistent throughout.
Do the comments make sense?
Code comments should be hints to the reader about what is going on. This isn’t the place for top-level design information. Comments should act as signposts to document why we’re currently doing what we’re doing. Since code is typically read many more times than it is written, handy implementer’s hints can be invaluable for future maintenance.
Comments are a good indicator of code quality. If a comment is out-of-date, or looks like it may have been copied-and-pasted from somewhere else (e.g. the comment is out of context), that may be a hint that someone wasn’t paying full attention when they made their changes.
Does the code have a good heritage?
One of the joys of modern source code management systems is that the history of each file in the code base is tracked. A developer can look back at how the code has changed over time, and this can be a valuable tool for estimating the quality of the code.
Bad signs include lots of change (which may indicate the design of the code is broken or fragile), poor changeset granularity (“big-bang” changes are easier to mess up and more difficult to test) and poor checking commenting (indicating the coder didn’t care enough or didn’t have time to write a decent comment).
Bonus points for documentation of why the change was made, the level of testing carried out, and references to bug/issue tracking tickets further describing the change.
Is the code doing anything ugly or stupid?
Every programmer knows that, every so often, one has to hack things a little to get them going. Whether you’re programming around a weird library bug, breaking an abstraction in order to get at some data you need, or adding an expeditious but aesthetically troubling function which you’ll clean up after this release is out the door (honest guv), sometimes a bit of outright hackery is the best way to get the job done.
The trouble with these little hacks is that, over time, they accumulate in a codebase to form technical debt. As with financial debt, you need to keep on top of it lest interest payments become prohibitive. This is partially what code refactoring is about: going back over the old ground to make good the things you missed or had to ignore the first time around.
If a code base contains a lot of hacks, it’s a good sign that the authors haven’t been able to carry out any refactoring work. This in turn suggests the code may be fragile, bug-prone, and difficult to extend.
Is there any evidence of code review?
As every competent coder should know, defects are cheaper to fix the earlier they’re discovered in the development cycle. In an ideal world, careful attention during system specification and design should minimise defects throughout the project. But in the real world, designs are not perfect. Even worse, programmers are only human and may introduce new errors of their own as they’re coding!
One well-understood method of catching errors at the coding stage is code review: formal inspections of work designed to catch programming faults. It is an effective way of improving code quality.
If there is evidence that a component has been subject to a code review process, it is far less likely to contain errors than an unreviewed component.