Raise the Bar, it’s high time!
It’s 2016, many people are still doing what they were doing last year, same old mundane and trivial tasks writing the same old boring piece of code with coding practices in the same old boring and inefficient way. People need to improve and change the way that they do things sometimes for fun other times for efficiency and quality, I have something that will get you all three for your team.
Let’s talk about “Code Review”, one of the most important coding practices and how to make it more interesting and effective and a way of learning new things every day.
DISCLAIMER: Many of you would be using the afore-mentioned techniques so no need to scream, I know you do, it’s an informative way of explaining things to you and people who don’t follow them already. It’s a personal view that these techniques help dev teams to a great extent, if you feel the other way feel free to ignore this (for lousy people nothing seems to help! ;) ).
Ok, so I’ll follow the classic way of explaining things the WWH (What, Why and How?) way.
TL;DR — Code reviews are more than just a review of the source code, it’s a review of the author, it’s a review of the reviewer, it’s a review of the business requirement and it always is the review of the team.
Code Review, by definition, is systematic examination (often known as peer review) of computer source code. But wait, there is more to it than just a Wiki article, nothing stated in that article is something that you may not know already. Code reviews are more than just a review of the source code, it’s a review of the author, it’s a review of the reviewer, it’s a review of the business requirement and it always is the review of the team. Let’s take them one at a time — A review of the
The author is the prime auditee who is being reviewed during this process. It’s his skills that get the heat of the reviewer and its code which has to step-in to the fire and come out pure and perfect to be absolved of any error. But, alas rarely is when this happens. There is always a chink in the armor, a complex piece without comments, a no sense making a variable name or something else. It is the author who has to make sure that he doesn’t slip into the trap of what I call the ‘Developer’s Illusion’ — A mental state when the developer codes for straight long hour and becomes delusional and sees himself and his code as invincible or he might miss a major part of the requirement because he was focusing too much on resolving a major issue or writing a complex logic.
The person questioning the ethics and morality in the code. It is very critical to have an open mind as the code review process is not only for the author but for the reviewer as well. I look at any piece of code like an ocean of knowledge, with new practices and viewpoints for approaching a specific target. While the reviewer should be the one giving insights I feel he is equally responsible to respect the author, his thinking and try to see author’s implementation as another approach and learn from it for his benefit. The review process must never turn into a mano a mano situation.
- Business Requirement
The reason for which the piece of code was put there in the first place. While questioning the code it is very important to keep in mind the requirement that the code fulfils and see if the amount of lines of code justifies the requirement and the requirement is actually a plus for the customer. If such is not the case then either you should ask the business to reconsider the requirement or refactor to reduce the amount of lines of code. This is just as important as any of the other standard issues with code.
- Team (most important)
One cannot ignore the fact that a code review with not so good comments (avoided use of ‘bad’ as many reviewees have much of an ego to start with) reflects poorly on the team with no exceptions. But there is also another side to it, I have always loved the quote — “Your team is as strong as your weakest reviewer!”, keep the quote in the mind as I’ll demonstrate further more on this later in the write-up.
IMPORTANT: Code review must not be performed only for the code that is application development specific but also for things like unit, integration, automation, manual tests etc. Anything that relates to the application must be reviewed before it is committed. This is a major misunderstanding when it comes to code review, make sure you don’t fall into the pit, a unit test gone rogue is just as harmful as an ill-reviewed piece of development code.
So, code review is all of this and much more but this covers the core of it. Now for the ‘Why?’ part of it.
TL;DR — “the average effectiveness of the design and code inspections are 55 and 60 percent”.
Yes I know it’s pretty stupid of me to discuss on the why part of this, but there is more to the why as well. In addition to the obvious set of benefits that code review offers there are few more that are not stressed enough or not discussed at all:
- Offers better understanding of the team members as they learn from each other’s perspective and thinking.
- Helps to maintain a level of consistency across the code base, design and quality.
- Helps achieve uniformity across team by making all members equally proficient.
- The uniformity then adds to benefit of gaining consistent quality even when there is a low-availability of members.
- Builds mechanical solidarity among the team members.
- Keeps technical debt on the team at low.
Apart from these awesome reasons, I would like to quote “Code Complete” — a fab text on coding prowess: “the average effectiveness of the design and code inspections are 55 and 60 percent”. I thinks this nails it for those looking for numbers.
This is where it gets interesting, this is why I thought of writing the article in the first place. The ‘what’ and the ‘why’ were just prologue to the ‘how’ of code review. Let’s get to it.
TL;DR — “Your team is as strong as your weakest reviewer!”
Just as before beginning to cook you need all the ingredients here too you need a set of tools that are essential for you to get through the code review process:
- Code review focus areas with their severities
A focus area would be something that your project need more attention to like Security, Performance issues etc. It may very well vary from project to project. So go ahead and decide for yours.
- Code review checklist
A checklist will typically contain a set of things to remember and confirm before you finally check-in that piece of code to the repository. These may contain things like author comments, remove build warnings etc. Checklist again may vary from project to project and even from team to team. Google it out, brainstorm with the team or go fly-solo and form a checklist for your team and project.
- List of team members ranked in their code competence across all development types and areas (I call it Reviewer grade list).
Now I’ll elaborate on this so as not to cut-across in a wrong way. These lists are more or less very volatile as they may get updated frequently when team members start to learn and grow. These are not meant to be a blow to the individual’s ego or a doubt on their capabilities. To understand it I would state a paradox: “people can hold different levels of power, and yet everyone can be powerful”. So go ahead a create n-number of lists listing your team members in the abilities. Each list could indicate a different development domain like UI design, Web services etc., you get the idea, and rate your team member in those domains. IMPORTANT: Discuss the same with the team and make necessary changes if the team wants so.
With these in place we can begin the code review process.
Code review starts when the reluctant fundamentalist (coder) starts firing away his fingers on the keyboard to produce a grotesque code. Now it’s too much debated if the coder should continuously review his code even when he is writing it down?, I stand for yes, yes he should as this will largely reduce the review process time while slightly increasing the development effort and as this becomes a force of habit the development effort will come back to what it was. For this phase of review the developer should use the developer checklist and keep in mind the focus areas being engaged in his development.
Once the development completes, the next phase of the review begins where the coder must visit the reviewer grade list to see who’s the weakest in the domain (excluding himself) and get him to review the code. This is what I call ‘the neophyte literacy’ phase where most of the smaller issues and defects should be eliminated. The best part of the this phase is that the weak reviewer will get to brainstorm with the coder and analyse defects if any, also this will give the reviewer a chance to see new perspective and discuss any other approach available to the problem.
Once the neophyte literacy phase completes the next and final phase begins what I call ‘the wise inheritance’ phase where the review is done by the best of the reviewer in the domain. This is where the more logical and functional defects and issues will get caught and more detailed insight can be provided to the fellow coders. This is a major knowledge inheritance phase for the fellow coders until they reach the same level of proficiency.
Now, to answer a few questions that I think might be raised like:
1. The number of reviewers in the last two phases?
I personally feel one reviewer per review is fair enough but it’s nothing that is set in stone, you can always mix it up as per your requirements.
2. Why to go through the trouble of two reviews? Why not just have a grand team review meeting?
This is something that I thought about a lot before beginning the write-up. Working in the agile process environment I feel team’s time is much valuable to be spent all at once and risk the review to still not be a polished one. In meetings such as this the number of successful and fruitful points to come up and get through varies inversely with the number of people present in the meeting, a sad but hard and true fact!
For further questions/suggestions leave them in your comments, I’ll be glad to help/learn.
Feel free to use/modify/praise this technique and as Captain Planet always said ‘The power is yours!’