Thursday, May 19, 2016

Should you follow the rules?

In the last article I wrote that I’m a big fan of tools for static analysis. Those tools help you follow established rules. To be more specific, they’re yelling at you through red builds whenever you break these rules. Isn’t it great? Before your code gets under the review, we are already sure that many things have already been checked and we don’t have to bother ourselves with verifying the code from this side once again.
However, this solution comes with a problem. When a developer receives feedback, they know what is wrong, but not always know why a particular part of the code was recognized as invalid.
And lack of understanding of rules can lead to huge damage.

What is a problem?

Recently one of my colleague from the team has tried to push code for a review. Yet, static analysis build didn't let him do it. It failed. The problem lay in a missing constructor for a util class. Ok, fix it and move on, but… after adding the constructor another issue was found - there was not even a single test for a private constructor. For the sake of coverage!

I don’t want to talk whether our rules are good or not. I want to talk about a feeling that may appear after some time of receiving such messages.
What’s the feeling I’m writing about? Irritation. Not because you’ve made a mistake in your code. It’s not a problem, everyone does. Not because build was red. It’s great, it should fail in case of our mistake. Not because we have to solve an issue. It’s reasonable that you are fixing issues in your code. The problem is that we learnt nothing. There is a message which describes WHAT is the problem, but it doesn’t explain WHY it is a problem.

Why it is a problem?

When you don’t understand the reason next problem found by static-analysis tool becomes annoying noise, not a good suggestion. You start looking for an exception from the rule, for an excuse to break it. This is not because you don’t want to create a high quality code. You do want, but until you don’t understand the reason behind the rule it may look stupid to you. It may look worthless.

What will happen if you don’t find an exception? Probably you will start to obey this rule. Automatically. But it will still be against you, it will bother you. And you won’t follow such suggestion, because you know why it is important. No, you will follow it because you have to. Such practice will be full of negative emotions. And if a new person some day asks you why you’re writing code in this way you will tell them that it is because “some stupid rules”. And now there will be two people who will apply it into their code without understanding.

Is it really a problem?

Is it really so bad? Isn’t it better to have a developer who writes good code without understanding rather than allowing them to write worse code?
Well, yes. If we are looking at this from this angle, this is true. However, we want to work with good developers, we want to spread the knowledge and understanding of good practices. We want to teach others and share experience with them. It will be a win-win situation for both sides. We want to work with people who can improve our solutions and challenge our ideas.
Over time, the quality of the code will get worse if we have to work with people who follow the rules blindly. Why? Because of the lack of knowledge. Code review, refactoring, design and redesign, etc. those are activities that require specific knowledge. If we don’t share it, there will be a huge number of developers who won’t do it because they wouldn’t know how.

Fix it!

After we fixed all issues in the code we had a great discussion about values of those static analysis tools. Values and the risks that I explained above.
When we talk about problems we start to think what we can do to avoid them. Or at least minimize. How to change a description into an explanation?
We come up with the idea to change default messages. Add explanation why a described problem is a problem. Or at least add a link. We should avoid leaving people without knowledge.
Our idea is to change those failing builds into a great lesson!

This is our idea and I’m pretty excited about it. I know it would take time to improve all messages, but on the other hand I’m also pretty sure it is worth the effort.

What do you think about it? Maybe you have your own idea? Share them in comments. It would be great to know your ideas.


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.

Monday, May 2, 2016

Need some explanation?

As developers we are doing everything what we can to increase the quality of the code we are working on. To make it easier, we learn to use new tools, techniques and practices. Each tool makes us stronger and better weaponized for the fight for the sake of the code quality.
However, a tool won’t be useful until we use it according to its design and intended purpose.
What are the main reasons behind doing a code review? In my opinion they include sharing knowledge (learning and teaching) and increasing readability and understandability of the written code.