Thoughts on Software Development, .Net, OOP, Design Patterns and all things cool
Code reviews are thought to be painful by many, but in my opinion that can be
avoided. Code reviews can be a great tool for a project to help keep the code
clean and concise.
Last time I talked about Code Reviews and how to make them successful, this time I thought I would spell
out some of the Rules of the Game.
- Critique the Code, not the Coder
This is the most
important rule for performing code reviews. The purpose of the code review is
to review the code, not review the coder. Keep everything on the positive. If
there is something in the code that is not right, make sure you speak only about
the code. It is also nice to give praise when you find something good, this can
go a long way as well.
- Check ego's at the door
Code reviews are not the time to
try to demonstrate who the better coder is. Actually, there is really never a
valid time to do this, but this is especially not the time.
It is
important that 'junior' level coders feel comfortable during the review and this
cannot happen if they feel like they are being belittled or looked down upon in
the review. This will also allow for a more open communication and will lead to
better results.
- Follow up on any changes
During most code reviews there
will be something found that needs to be changed/updated/removed/fixed/etc.
Make sure that these get noted. A buddy of mine likes to leave TODO's in the
code with the date and the change needed. I live this approach because you can
easily search for these and fix them.
It is important to not only mark
the changes, but to follow up to ensure the changes have actual been made. I
would suggest that the first 5 minutes of any code review be spent reviewing the
'TODO's' from the prior code review session.
- Submit your code to reviewed prior to the review
In
order to keep the code review short and sweet it is best to send over the code
to be reviewed before hand. This also allows the reviewers to be familiar with
the code. If you have any design docs or business docs, send them over as
well
If the reviewer of the code is familiar with the code before the
actual review they will be able to ask more intelligent questions and should be
able to speak to more about the design of the code. As one commenter said
'doing a code review without understanding / approving of the design of the
code is bad. The code review will end up devolving into "you forgot comments
here", "you used the wrong parameter naming convention here", etc. Those are
valid points for a code review, but they don’t really get to the heart of what
you’re trying to accomplish with a code review.'
- Keep the review short and sweet
Try to stick to a 30
minute time limit for the reviews. Keeping the review short and sweet will
accomplish a few different things.
1) Will keep everyone on track with
the task at hand.
2) Allow for a more productive meeting.
3) Will allow
for more reviews, if they only take 30 minutes then you can do them monthly
- Create your own Rules for the Review, what are you looking
for
Before you even have the initial code review, there needs to be
your own rules or check list as to what is to be accomplished during a code
review. This will allow the author of the code to know what to expect before
the actual review as well as keep all the reviews consistent. The worst thing
that could happen would be for each review to be different because then no one
would know what to expect and the usefulness of the reviews is out the
window.
Potential Rules for the code reviews (in no given order)
1) Is
the code following coding standards?
2) Is the solution solved by the
simplest means possible?
3) Are there unit test for your code?
4) Are the
proper/valid comments about the intent?
5) Did you solve the actual business
problems?
6) .... etc
- Ask questions to the coder, don't make judgements
If
during the review there is some code that is questionable make sure to ask the
author questions rather then make judgements or statements. By asking a
question, you will avoid a defensive response from the author. This can help
relax the author and can often times open their mind and allow them to see your
angle better.
Even though you are asking questions, and not making
judgements during the review. Stay mindful to not ask the 'Why Did You Do
This?' question. It is ok ask what the intent was for the code, but make sure
to phase it in a positive light.
- Understand that there is more then one way to solve a
problem
Software development is pretty unique in a lot of ways and
one of them is that there are many ways to solve a common problem. Some better
then others, but still many ways. If the author of the code chose a way that is
different, that is fine. The goal is to make sure the code is quality and
maintainable. If this solution is solid and the code is of good quality move
on.
If these rules are followed, code reviews can be relatively painless and
very, very successful.
About Derik Whittaker
Derik is a .Net Developer/Architect specializing in WinForms working out the northern suburbs of Chicago. He is also believer and advocate for Agile development including SCRUM, TDD, CI, etc.
When Derik is not writing code he can be found spending time with his wife and young son, climbing on his bouldering wall, watching sports (mostly baseball), and generally vegging out.