Keeping clean: why coding standards are important

June 12, 2014

Recently I had to investigate a strange bug that only happened on our pre-production environment. Code that was fine all through our other development and test environments was causing a white screen of death on the home page. Not good.

We’re a big team, and we have quite a strict code review process, so in theory no code should get into the main branch unless it’s had at least two pairs of eyes on it, and nothing should get through to pre-production unless it’s been tested in two other shared environments, plus the developer’s local machine.

The obvious thing to look for was configuration differences between the environments. Different PHP version. Facepalm…

So where do you start? You can’t look at the change notes on the PHP project and then search your code for all the things on the list.

Luckily, I was using PHPStorm, which includes a code inspection tool that allows you to set the PHP version.

Within a couple of minutes, I’d found the problem, or rather the tool had found it for me, in some fairly innocent-looking code:

$output = !empty($object->getAdvisedStatus()) ? t('Advised') : '';

Because there weren’t an enormous number of warnings, it didn’t take long to see the relevant issue, sitting in the inspector under Language Level “Arbitrary expressions in empty are allowed in PHP 5.5 only”, with a link to the relevant file, where I could see the offending code underlined in red.

There are a few things to learn from this:

  • make sure you test before you release to production
  • keep your development, test and production environments as closely matched as possible
  • keep your code as clean as possible

These should be obvious, but it’s easy to overlook them.

Why bother to keep your code clean?

Because it’s easier to read, and that means it’s easier to see when something is wrong.

It’s easy to ignore or turn off warnings about coding standards, especially when there are hundreds of them. They’re overwhelming. Besides, who cares that your comments don’t end in a full stop?

But that’s the point. If there’s too much noise, it’s much harder to find the signal.

Similarly if your server logs are full of PHP notices about things like undefined array index, it’s easy to miss the significant stuff that tells you what’s really going on.

Besides, there’s a performance hit from logging lots of warnings.

The thing that goes along with this is that you should always have your editor set up to display advice from code quality tools, like code sniffer and mess detector.

PHPStorm is great for this, because the warnings are just conspicuous enough. You notice them, and register that you should do something about them, but you can ignore them and crack on with more important work.

The other important thing to bear in mind is that you should set this stuff up before you start. It’s much easier to develop following coding standards from day one than it is to tidy things up later, when everything will need to be regression tested.

Carpenters and other craftsmen will start by setting up their workspace properly, and developers should do the same.