Derik Whittaker

Syndication

News


Images in this post missing? We recently lost them in a site migration. We're working to restore these as you read this. Should you need an image in an emergency, please contact us at imagehelp@codebetter.com
Code Reviews: Rules of the Game

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.


Posted 06-12-2007 8:41 AM by Derik Whittaker
Filed under: ,

[Advertisement]

Comments

Louis Haskett wrote re: Code Reviews: Rules of the Game
on 06-12-2007 10:54 AM

My 5 cents on a rule like "Is the code following coding standards?":

Your coding standards should basically be however you setup a tool like resharper to reformat your code.  I.e. why waste everyone’s time with wrong spacing or brackets or whatever.  Automate it.  That will take care of 80% of coding standards stuff.  Leaving you time to catch the 20% that matters, like some code ninja using the variable 'X' everywhere.

Nick Ohrn wrote re: Code Reviews: Rules of the Game
on 09-24-2007 6:49 PM

Hey, this is a great article, especially for someone just starting to incorporate code reviews into their practice.  Thanks for writing this.

About The CodeBetter.Com Blog Network
CodeBetter.Com FAQ

Our Mission

Advertisers should contact Brendan

Subscribe
Google Reader or Homepage

del.icio.us CodeBetter.com Latest Items
Add to My Yahoo!
Subscribe with Bloglines
Subscribe in NewsGator Online
Subscribe with myFeedster
Add to My AOL
Furl CodeBetter.com Latest Items
Subscribe in Rojo

Member Projects
DimeCasts.Net - Derik Whittaker

Friends of Devlicio.us
Red-Gate Tools For SQL and .NET

NDepend

SlickEdit
 
SmartInspect .NET Logging
NGEDIT: ViEmu and Codekana
LiteAccounting.Com
DevExpress
Fixx
NHibernate Profiler
Unfuddle
Balsamiq Mockups
Scrumy
JetBrains - ReSharper
Umbraco
NServiceBus
RavenDb
Web Sequence Diagrams
Ducksboard<-- NEW Friend!

 



Site Copyright © 2007 CodeBetter.Com
Content Copyright Individual Bloggers

 

Community Server (Commercial Edition)