Embrace Your Worst Critic

manyreviews

How tiresome are linters? The worst experience I had with a linter was when I had to wait until the end of a 5 minute build on a CI server to discover whether I’d forgotten to put a full stop in some JavaDoc that nobody would care either way about.

When a linter is employed in a way that’s unclear and late, they seem to generate work. This is especially true if you work in big batches, where suddenly several hundred lines of code are being declared wrong by a poorly configured piece of software.

Despite this, I’m going to advocate using static code analysis and making it a part of your process.

I’ve long used SonarQube to advise on code quality. I’ve used it in two modes:

  • Advisory – see if it comes up with anything interesting, and maybe do something about it
  • Critical part of QA assessment – attempt to reduce all warnings to zero.

What I’ve discovered is that the more you explore a correctly configured SonarQube, the more you end up making it a critical part of the QA process.

It’s my experience that the issues raised by SonarQube are not necessarily the cause of a problem, but second or third degree consequences of something that could be clearer/neater/safer. It’s always worth digging into the issues and resolving them if time permits.

Sometimes linters get it wrong and they can have those warnings disabled (with explanation, so the next person understands why).

However, all of this discussion so far hasn’t addressed the post-change churn that we want to avoid. Hitting build shouldn’t be the start of an argument with your linter.

Luckily, these days most linters can be integrated into your IDE for realtime feedback. Where you can do this, do it. Get your IDE to show the code is green before you even try to build it. Fix issues as they arise. Learn how to write code your coding standard agrees with.

If you can’t build it into the IDE, then it should be a part of your local build. Work in small batches and you’ll only have a few tweaks to make.

Here’s what you get out of linting:

  • Critical feedback on the risks of the code – things like FindBugs can warn you of surprising side-effects of dodgy patterns
  • Standardisation of your code – the coding standard stops being a guide, which each developer does their own way – and starts to be enforced
  • A second pair of eyes on fresh code – if the linter doesn’t like it, does that mean that you’ve been doing something you didn’t think you were doing? Would it be better structured differently
  • Less stylistic discussion during the peer review stage – if the linting is enforced in the build, you don’t need to discuss code style as much during Pull Request review etc.
  • Holding yourself to a standard – in terms of mastering quality software development, the process of maintaining a standard, provably, is an excellent discipline.

If you don’t have linting at present, then you can introduce it. You may wish to reduce the number of warnings resulting in a build fail until all your current issues are resolved. Turn rules on a few at a time to incrementally work through legacy lines of code. Or bite the bullet and cleanse the whole project, so you’re starting fresh.

I am divided in my own opinion about making builds fail on things like punctuation, order of imported resources, indentation and the myriad other minor grips that Checkstyle and their ilk come up with. Perhaps making the build fail is a step too far, but listening to a linter as part of development, as early as possible, is a great idea.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s