How to Conduct Effective Code Reviews
Join the DZone community and get the full member experience.
Join For FreeI have always felt that code reviews are very important in any software project. More often I happen to see only "superficial" reviews happening. By superficial, I mean the types where you get review comments like, "You know the documentation for that method doesn't have the version number", or "this variable is unused", etc. Although this is an essential part of any code review, it doesn't cover the subtle aspects of a real code review. It takes more than just a static code analyzer to perform a good code review.
In this article I have tried to share with you a few points on how to make code reviews really effective.
1. A good code review always goes beyond the obvious
This is one thing that makes a code review really effective. Any static code analyzer can tell where you have violated a naming convention or where there are unused variables. But it takes a good programmer to point out subtle improvements or even bugs in a code. If you have been entrusted with the responsibility to review some code, you better to bring out stuff that skips the 'eye of a static code analyzer'. A good example could be to point out the improper usage of an API (either one from JDK library or a custom API) and propose an alternative to the existing one
2. Try to avoid becoming the bottleneck in the review process (I am not a big process weenie!!)
Most often I have found that if someone in a team is doing the code review, he/she gradually becomes the bottleneck. This has more to do with attitude than with anything else. It's not a great idea to have just one person perform code review. It would be bad for 2 reasons.
• It kind of puts the ownership of maintaining the code quality (read it as maintainable code) on the reviewer. He/she could even be victimized for lack of code quality, if there is just too much of code to be reviewed.
• I feel that not sharing one's valuable experience in programming with peers and junior members kind of invites dependency on one single person and deprives others of a chance to pick up similar skills and become self sufficient.
So in the best interest of the team, make sure you gradually pull in a couple of people in the team and have them with you as go over a piece of code and let them get a first hand feel of what a code review is about. I am sure if you can manage to repeat it with a healthy rotation, you would have a more informed team and also a lot of "pressure" on you gets taken off.
3. A process is just an enabler not the end in itself
I always feel this whenever someone talks to me about process and process and process.... It kind of gets to me. I just feel like asking them one question. What do you really want to achieve? Once I get to know what they want, I begin to think... ok what are the possible ways of getting there. If you observe, the focus here is not on "how to get it done?" rather it is on "what is to be done?” This is because there can be n number of ways to do a thing.
I see that a lot of times code review gets limited to just sharing those jazzy dashboard reports which you send across the client (even if they can't make sense out of it), simply because it is "part of the process". For god's sake spare the code review from getting crippled by this conventional process stuff. An effective code review can still be done without the so called "processes" and "audits". It can be done by making honest efforts to make sure the team is educated of ways to program better than to scare them with facts and figures or worse hint them of their implications on their appraisal.
The real purpose of a code review is not to blame team members and show them how bad a programmer they are, but to make them aware of how to write better programs. It should always have a positive outlook. If such an initiative calls for choosing unconventional ways of conducting code reviews, please go ahead!!
4. Build the culture of knowledge sharing in your team
Promote the culture of knowledge sharing in the your team. It helps in keeping the overall knowledge quotient of the team high. It is very important to facilitate knowledge sharing inside your team by creating a lot of opportunities of sharing through multiple channels like wikis, blogs or tweets etc. An alternative could be to have groups going through a technical book and share the collective knowledge with the team. It can go a long way in gradually making the team self reliant and less and less dependent on the actual code reviewer.
5. Rome was not built in a day. Take your time
It is always easier said than done. I have been in a couple of development projects and I know how deadlines can really take the focus off the once-so-cool-goal of let’s-write-a-rocking application. It really can push even the best teams to compromise on the code quality (quality as you know does has something to with time and cost as well). But the biggest challenge is to never give up that desire to consistently strive to keep the code just healthy enough.
It’s like your body. You can never escape from occasional illness. You are bound to fall sick. But it should not stop you from striving to lead a healthy life style. A code base too, often has its own cycles of illness. You would always have modules in your code base that just couldn’t match your golden standards. Never mind! They should just be there as exceptions and be ‘cured’ when time permits. Having patience is the key. There is a very thin line between what is real and what is practical. So next time you see a code that hasn’t met your standard, do walk up to the developer and understand why he/she coded a module in a certain way. If it is a genuine lack of time issue, you will know it for sure.
The key is to gradually prepare your team so that it can progressively write better and better code as they go through successive cycles of development.
<> Last but not the least, it is very important to never have your eye taken off the real goal, which is to deliver a code that does what it should, and reaches the client’s kitty when it matters to them the most!!
<> <>Opinions expressed by DZone contributors are their own.
Comments