Coding with strict inspections as a form of learning

I am a strong advocate for coding to very strict guidelines.

When I first started programming, I did not come from a computer science background. In my first job, after a year of programming in C, I joined a team working remotely from each other on a product which was an acquisition of an acquisition. The product was in Java and the previous team had all left.

There was no strong Java expertise in the team, and I had no-one in the same office with whom to learn together. I had one very knowledgeable friend in another team I could call upon in times of need but I didn’t want to abuse that privilege. I wanted as far as possible to try and write good Java so I turned to my IDE for help.

Turning everything on

I spent a little time going through every one of the IntelliJ inspections (it was version 6 or 7 at that point) and turned on anything that sounded at all sensible. These were very stringent and included warnings for overly complex/long classes, overly complex/long methods, too many parameters, too many dependencies, too much nesting, and so on.

Learning from the documentation

I learnt an enormous amount just from reading through the inspections. Each inspection came with a description of what it did and why you might want to do it, which broadened my knowledge considerably.

Staying motivated in the face of all that yellow

Unfortunately, the product I was working on had a large codebase, had originally been written many years before, and had seen many hands and technologies. When I first turned on the inspections, I was confronted with walls of yellow warnings and IDE performance degradation on some of the bulkier classes.

This could have been dispiriting but I took it on as something of a challenge: this is what the existing code might look like, but my code, new or refactored, would stand out in contrast. A decent number of the warnings, especially the noisy ones, could be removed using IntelliJ “fix all” commands, standardising syntax across all files. This was a necessary precursor to making any sensible headway when working with the existing code.

The simple stuff

A lot of the warnings correct potential bugs and keep syntax consistent. These help you get the simple stuff right and help write consistent code, so a newcomer to the codebase feels equally at home in any of the classes you have written.

Prompting timely refactoring

I think this is incredibly valuable, but what I have been most impressed by in retrospect is the value of those more elaborate inspections picking up on code complexity and coupling. With a mature code base, the likelihood is that by turning these on you will have to live with a lot of warnings that cannot easily be fixed. If a class has twenty dependencies it probably isn’t that easy or quick to refactor. However, if you abide strictly by the rule that you will introduce no new instance of one of these warnings it promotes a lot of timely refactoring. Time and again, I was prompted to refactor and extract logic, interrupting an otherwise simple task adding new logic, and time and again I was later grateful for having done so when I revisited that area of code again.

I was prompted to refactor before the code became a problem, just as it was beginning to smell, and that made it easy. It also got me thinking about architecture and patterns to avoid these problems from an early stage. Different teams likely have different ideas about what constitutes too many parameters or too many dependencies, but most people would have a limit and I think that codifying that limit in an inspection is a good idea.

Sharing with the team

When it comes to writing clean code, or keeping code from degrading into code which is hard to understand or maintain, you generally need to make sure the “right action“ is the easiest action. For me, I put sufficient weight on the warnings that doing the refactor was easier than justifying the new warning to myself.

Initially, I was operating entirely alone with this practice. Apart from anything else, half the team didn’t have access to IntelliJ for a long time. The rest of the team were always interested though, and as soon as others were on IntelliJ, I committed my settings to source control, and we all used them. This helped us all write code in the same style and also helped reduce the number of trivial quibbles which might arise during code reviews. The inspections were especially useful when a junior developer joined me on the team in my office and I was tasked with brining them up to speed. Having more people involved also provided more feedback on which inspections were working and which ones weren’t, and we adjusted accordingly.

Bringing automation into it

When we first added our products to a build server, I found that we could run and report on the inspections as part of the build. Being able to have a UI with metrics for the inspections which all the team could view was great. Being able to fail the build if there were inspection warnings was even better, as it prevented oversights and helped keep everyone honest.

For that set of projects we only failed the build on warnings for newer code bases where we had no warnings. On subsequent teams, with mature code bases we started by adding a failure if the (non-zero) number of warnings increased. There can be some frustrating edge cases when doing this, depending upon what branch is being compared with what, but on balance I still think it is very worthwhile.

Not every inspection is worthwhile but most are

Not every inspection I switched on when I first went through the IntelliJ list was a good idea, at least for our code style. In some cases this was immediately apparent, producing far too many clear false-positives from our perspective, and with some it took a little time to work out. But the vast majority did prove to be good news. Just because some were occasionally invalid, was not a cause to dismiss them. It is always possible to suppress a warning, if it is done very judiciously and a clear explanation is provided.

Coding to these strict guidelines has stood me in very good stead and I still look at the list of new inspections whenever a new version of IntelliJ or Rider comes out to see what people have thought might be a good idea to enforce.