Our Code Quality Checker rocks

2,294 views
Added 02 August 2009, 3:45 AM

One of our best tools for hunting out bugs before a release is our Code Quality checker. It's a rather sophisticated tool we wrote a few years ago, and have maintained over the years, and it can find all kinds of problems that might have otherwise gone unnoticed. I'm not going to explain the CQC here, but anyone interested will find it is explained and available from our Code Book (linked to from our Support section).

Here's a problem it just picked up in our 4.2 code:

Code

if (substr($avatar_url,0,strlen(get_base_url())==get_base_url())) $avatar_url=substr($avatar_url,strlen(get_base_url()));

The error was:

Comparators must have type symmetric operands (integer vs URLPATH).

This is a great example of how our code standards help us locate various bugs before a release can happen. Most people in the PHP developer community would think we're anal and insane to have our code-base written to be "type strict", as it goes against the grain of the PHP language. However it is incredibly helpful because it gives us discipline and allows us to detect bugs. The bug above was a misplaced bracket, the code should have been:

Code

if (substr($avatar_url,0,strlen(get_base_url()))==get_base_url()) $avatar_url=substr($avatar_url,strlen(get_base_url()));

Without this bug fix, members re-imported via CSV files would lose their avatars if the site ever moved to a new domain name, as the code that kept the avatars relative to the site's base URL would not have functioned. The only way to notice this bug would have been if you'd moved the site, and that isn't something you'd typically test. Of course, we could have manually test the code path, and we probably should have – but this one got missed.

Writing type strict code isn't just about reducing bugs by maintaining extra constraints to check for. It's also about discipline. A programmer who properly understands type conversions is a better programmer, who will write more stable, secure, code, that is easier to internationalise.