Skip to Content

Overview

Wouldn’t you like to have a tool that helps you with verifying the code you or your colleagues are writing, helps you in finding code faults, helps you in knowing the code that is being developed near you (in other teams/developers), helps you learn new API’s and development techniques, helps your organization with forcing a well defined development guideline, etc….? Well there are some technical tools for achieving some of the above mentioned goals but there’s one famous process for that that is called Code Review. The most interesting thing in Code Review is that everybody knows it (or at least heard about it) but too many developers don’t do it. During this weblog I will try to describe what a Code Review is and its importance for any development team (from the smallest to the biggest), and any programming language, and how easy it is to do it.

What is code review?

Code review is part of the Quality Assurance process regarding programming. This process is a consistent task that is being made in parallel to the development process. During the code review we are going over the code that is being developed in order to make sure that the code is well written according to our industry standards. The code review is being done by other developer/s (or other role with language and coding standards experience) and not by the one that wrote the code. It is recommended that in the code review we will have a number of participants with different programming skills. This process is important both to small and big development teams/organizations and it saves time and effort to the organization in several ways:
Reducing bugs – by reviewing the code we can identify bugs before they are shipped to production (or other testing environments). During the code review it is very easy to identify mistakes the developer missed.
Improving the code performance – usually in code review we have strong and experienced developers that can easily identify bad practices, inefficient code and make suggestions for improving the code.
Knowledge Transfer – share knowledge between the developers within the same/other development groups. By reviewing each other’s code the developers can learn from each other in the sense of what to do and what not to do.
Better maintenance
 &nbspo By reviewing the code, the developers know each other’s code and can maintain this code if needed (in case of personnel changes).
 &nbspo During the code review you can make sure that the developers are writing the code according to the organization/industry standards that will make sure that the code is easier to maintain.

What should we review during a code review?

During a code review we will check the following:

Coding according to the organization’s coding conventions – every development team/organization should have its coding conventions; the code review process is the process that helps us make sure that our developers are applying these conventions. Here some examples:
• Does the source file start with an appropriate header and copyright information?
• Are variable declarations properly commented?
• Are all functions, methods and classes documented?
• Are function parameters used for input or output clearly identified as such?
• Etc
Error Handling – error handling is a very important part of coding. A lot of bugs are made because of bad error handling. Here some examples:
• Are errors properly handled each time a function returns?
• Are resources and memory released in all error paths?
• Are all thrown exceptions handled properly?
• Is the function caller notified when an error is detected?
Resources handling – resources handling is a major problem with coding, we often see programmers forgetting to release resources or missuse them in a way that can damage the code performance or, more dangerously, the all system stability. These kinds of errors can affect other developer’s code as well. Here some examples:
• Is allocated memory freed? Even in Java we have to make sure that the code is written in the proper way in order for the Garbage Collector to handle this memory as it should.
• Are all objects (Database connections, Sockets, Files, etc.) freed even when an error occurs?
• Is the same object released more than once?
• Reuse of resources (Database connections, large data objects)
• Creation of unnecessary objects (like in loops).
Thread Safety – in our code review we have to make sure that the code is safe from threading perspective. Here some examples:
• Are all global variables thread-safe?
• Are objects accessed by multiple threads thread-safe?
• Are locks released in the same order they are obtained?
• Is there any possible deadlock or lock contention?
• Can we use threads (for example on J2ee – EJB)?
• Are we locking in the right scope (can affect performance)?
Control structure – we need to make sure that the control structure is being done properly. Here some examples:
• Are loop-ending conditions accurate?
• Is the code free of unintended infinite loops?
• Are if statements defined accurately?
Performance – we have to make sure that the code is written in the optimized way and will have the best performance we can achieve. Its is very good practice to first code from the functional perspective , and then review and try to see how can we implement the same functionality with better performance and efficiency. Here some examples:
• Do recursive functions run within a reasonable amount of stack space?
• Does the code have an impact on size, speed, or memory use?
• Are you using blocking system calls when performance is involved?
• Is the code doing busy waits instead of using synchronization mechanisms or timer events?
• Was this optimization really needed?
And more – I’m sure you can think of additional topics that need to be checked during the code review.

How should we do it?

There are a number of formats that I’ve encountered during my development time to do a code review. Here’s one way to do it that I found very useful:
• Reserve a fixed day and time for making a code review session, the frequency should be determined by the project size and time table.
• Appoint a code review officer that will manage those sessions.
• For each session invite:
 &nbspo The developer whose code you are going to review.
 &nbspo Team leader/senior developer.
 &nbspo A few developers both from the same and other teams.
• The group should not be too big (not more then 10 people).
• Appoint one of the participants to document the review.
• Create a code review session report document – this document will be filled on each code review session.
• During the code review the developer will describe its code logic.
• During the review each one of the participants can ask questions and suggest alternative code if needed.
• During the review every participant has the same rights(even if he’s new to the company or in work).
• Every suggestion should be discussed during the review in order to decide if the change should be made.
• Every decision for change will be documented in order to make sure that it won’t be forgotten.
• After the code review the developer can get the document with all the change decisions and start work on that. Its team leader should make sure that all the changes are made.
• It’s good to send this code review session report to all the developers in the organization so they can learn from it.

Summary

The code review session is a very simple process that contributes to the organization in many ways. Though it’s simple I see too many organizations that do not apply it. In order to make sure that our code review process will succeed we have to remember that:
• It’s important to make sure that every person that writes code will be reviewed.
• It’s important that at least more then 40% of the code will be reviewed (it’s not realistic that 100% of the code will be reviewed).
• It’s important that every person that writes code will participate in other’s code reviews (as much as he can). That way we make sure that our developers will learn from each other.
• It’s important that this process will be sponsored by the management.
• It’s important to include this process in our work plans (this process takes time and resources that have to be taken into account).
• It’s important to share each session’s results with the whole group (spread the knowledge).

I hope that during this weblog I’ve managed to explain how important this process is, and even though it looks complex to organize it, I can assure you from my rich experience that it’s quite simple and very profitable to the organization as a whole and for each person in it.
Enjoy

To report this post you need to login first.

9 Comments

You must be Logged on to comment or reply to a post.

  1. Former Member
    Nitzan…very good blog.  Even more pertinent now that development is moving closer to the customer with the merging of applications and infrastructure.  How do we ensure these practices are adopted when developing business solutions powered by SAP NetWeaver?  Also what impact do tools such as the NetWeaver Developer’s Studio, CAF and Visual Composer have on this?

    Thanks.

    Dan

    (0) 
  2. Former Member
    Nitzan…very good blog.  Even more pertinent now that development is moving closer to the customer with the merging of applications and infrastructure.  How do we ensure these practices are adopted when developing business solutions powered by SAP NetWeaver?  Also what impact do tools such as the NetWeaver Developer’s Studio, CAF and Visual Composer have on this?

    Thanks.

    Dan

    (0) 
  3. Former Member
    Nitzan…very good blog.  Even more pertinent now that development is moving closer to the customer with the merging of applications and infrastructure.  How do we ensure these practices are adopted when developing business solutions powered by SAP NetWeaver?  Also what impact do tools such as the NetWeaver Developer’s Studio, CAF and Visual Composer have on this?

    Thanks.

    Dan

    (0) 
  4. Former Member
    I personally see Quality observed by letter and not in spirit. People do black box testing religiously but rarely the white box. Code review is looking under the hood and is to my mind one important activity to deliver quality robust software.

    Article is excellent.

    (0) 
  5. Former Member
    I personally see Quality observed by letter and not in spirit. People do black box testing religiously but rarely the white box. Code review is looking under the hood and is to my mind one important activity to deliver quality robust software.

    Article is excellent.

    (0) 
  6. Former Member
    I personally see Quality observed by letter and not in spirit. People do black box testing religiously but rarely the white box. Code review is looking under the hood and is to my mind one important activity to deliver quality robust software.

    Article is excellent.

    (0) 

Leave a Reply