Thursday, March 06, 2008

Code review thoughts

Yesterday we had a very good session of code review on a project we are working on (developing a solution that updates Active Directory when the user updates his user profile) so I figured it wouldn't hurt to share how we did it.
Code review, if done right, is a great process to raise the quality of the code and therefore the end result of what you are developing.
So, how did we do the code review? We sat in a room, all developers, and opened VS on the projector. After a short explanation of what the code is trying to achieve, we just walked through the code. We let the most junior developer read the code to us and explain what it does (don't just read out the lines - make sure he translates the code into pseudo code - for example: "we loop over the user profile's properties and put them in a string array"). During this, everyone of us was in charge of looking for way to improve the code:

  1. Naming conventions
  2. Coding standards
  3. Optimising performance
  4. Error handling
This way you have several people thinking and double checking, and you end up with 6 developers knowing the insides of the code instead of 2 (for example).
Some rules:
  1. Keep the atmosphere light and not overly criticising - we are here to get better.
  2. Make people who didn't write the code read out and explain the code.
  3. Involve everyone in decisions on how to optimize the code.
  4. Go through the code in a logical order - start from the things that the user sees, and walk through the code to the backend code.
These are my thoughts. How do you do code review?


FredMorrison said...

Code Review? What code review? Seriously, since I'm the only one on staff that knows anything about the type of development I'm involved in (SharePoint custom workflows using VS 2005), there isn't anybody who can do more than look at variable names and method names. It shouldn't be that way, but it is.

Anonymous said...

Wouldn't the process of code reviewing be much quicker if the code was reviewed on the fly, rather than having a whole meeting to do it...? I don't know how much time to have to develop, but I know I'd love to be able to put aside time to go and book a room and review code like that!!

I'm unsure about the benefits of making the least senior person go through the whole code too, everyone no matter how "senior" a developer they think they are should share responsibilities and roles... Less senior developers can still bring something to the table.