Thursday, September 30, 2010

Effective Code Review - A Presentation

(Cross post from IRefactor)

Here is a short presentation I did a while ago on a Code Review process.

I am sure, there are a lot of different ways to do code review, but the following served me well in the past.

One important note to bare in mind is that a Code Review cannot take more than a few hours.
People get tired easily; It's difficult to read somebody else's logic (code) and even more: to understand and to pinpoint the problems.

Therefore the Code Review process should be effective and highly cohesive as much as possible; I like the following metrics:
  • Summary: Ask a Software Engineer you are reviewing to explain in bullets what are the purposes of
    the application, the module or the feature you are going to review.
  • Top-Down: Require to review high level components; High Level Design (Class Diagram, Component
    Diagram, Some other Sketch and etc...)
    Understand in general what are the main components / classes in the solution.
  • Focus: Ask the Software Engineer to point you towards the most problematic sections of the code.
    He is the one who knows best what was hard to design or to implement.
    Ask him to explain what was the problem and how he tried to resolve it.
  • Bottom-Up: Now start from the code you focused on (You can also review the Unit Tests in order to
    understand better the code's behavior).
  • Read and Communicate: Take an ownership on the keyboard and observe the code. Try to see whether it is easy to understand the code and to navigate through it.
    During your drive you should communicate loudly what are you doing; Where you are going next (e.g. "I'm opening class X"), what you think of a piece of code you are reviewing and etc... You should ask questions in order to understand why it was implemented the way it was and also to verify that other alternatives were considered by the Software Engineer.
    You should spot the following:

    • Bad naming (classes, methods, properties and etc...)
    • Code Smells
    • Design Violations (SOLID, GoF and etc...)
    • Architectural Flows
    • And many others (I won't discuss here currently)...
  • Your mutual notes should be written by the reviewed Software Engineer (navigator) and fixed later on. Don't waste your precious time to tackle immediately the problems.

No comments:

Post a Comment