Sunday, May 8, 2016

What’s really matter? When you break a rules!

I’m a big fan of tools like PMD, FindBugs, checkstyle, etc. Those tools are really helpful. Thanks to them you don’t need to worry about many small issues. You don’t have to verify whether an author of the code you are reviewing follows coding standards or other agreed practices. Your tools will do it for you.

But do you know what’s really great? It’s not the fact that we don’t have to spend our time on checking coding standards. In my opinion, the best thing is that using those tools makes more visible each situation when someone wants to abuse or break a rule we agreed on. And this is exactly what we are interested in!

How does it work?

In our project we agreed once that forbidding using too many dependencies in the class will improve our code and will force developers to focus more on design. We agreed on 4 dependencies and we wrote a PMD rule that protects us from breaking this rule. From that moment, if you write new code or introduce a change which results in bigger number of dependencies, our static analysis will fail and there will be no possibility to merge this code into the code base until you fix it.

But what in case of an important fix that has to be introduced fast because without it performance of an application is not good enough or business requirements are broken? It sometimes happens. And if we don’t have too much time before release we are OK with putting temporary not-so-great code to our code base.
However, when you are using tools that protects rules from breaking them you have to turn checks off for this particular class for some time. You have to do it to make all builds green. And this is something that definitely will be noticed in the review. Why? Because it will be derogation from the rule. Something weird. Something unexpected. Isn’t it great? This is exactly what needs to be challenged by the reviewer. This is exactly the part of the code that needs a solid justification.

We don’t care about each space or bracket, but we care if someone violates our rules. We want to make such changes as visible as possible.

Don’t leave the code

One important thing is that you have to become responsible for a change that breaks a rule. You have to fix it as soon as possible. That’s mean you have to put it with the highest priority on your to do list.
If you won’t fix the code just after everything will calm down a bit you will introduce an exception from the rule and it may lead to many problems.

You let yourself broke a rule? You justified your decision and convinced someone that code should be merged?` Ok, it is sometimes what we need. But take the ownership for this change and fix the broken code!