Code review is the process of letting a reviewer, or a person other than the original code developer, inspect or review code (though some developers do review their own code before checking their new changes). In the book ‘Code Complete’ by Steve McConnell, code reviewing is explained as one of the cheapest and most effective ways for finding bugs and improving code quality. Kevin Burk summarizes McConnell’s most effective methods in his blog that we would highly recommend reading.
Code reviews can be done both formally and informally, in different steps (of software production life cycle), and by either developers (a senior developer with a QA manager role, some senior developers arbitrarily, or developers in general) or by members of a QA or QC test team. Large and disciplined teams in companies with more resources try to practice formal code reviewing, which was broadly introduced by IBM’s Michael Fagan in 1986 and has 7 different steps practiced by 3 or 4 people.
As an alternative, there is lightweight code review, which is more popular in agile teams because it is less expensive (both in time and other resources) and it still is an effective method to decrease the number of bugs and improve the quality of source code. Usually, lightweight code review is done by practicing pair programming, over-the-shoulder code discussion and inspections, sending code via e-mail and asking for reviews, getting the help of some third-party tools that allow developers to share and give feedback, or by a mixture of these methods.
Performing code reviews in an ad-hoc, informal way is also effective because it lets developers adopt the best method for themselves and their teams based on their culture and resources.
A team can start by setting this rule: Don’t deploy unreviewed code into production (as you never deploy code without testing it), and the code review methods should develop organically from there.
Queue-it & code review
Queue-it’s product, an online queuing system, is composed of different parts. The queue engine module is a critical piece of software that runs 24/7 and handles huge amounts of requests per minute by end-users (10,000 requests per minute) in a distributed environment. It has the responsibility to queue requests coming to different resources (i.e. a web page) and de-queue requests at the right time and serve them (redirect them to their target URLs).
For Queue-it developers, it is important to have as few bugs and errors as possible that might disturb this process (queue service should have a 99.9% working time as a cloud service). In our software development process, besides having different test types (Unit, UI and integration testing), we have a strict but informal procedure for reviewing codes: code changes should never go to production before they are reviewed (even for small bug fixes).
For us, it is important that code reviews are not just a senior team member reviewing a junior team member’s code; code reviews should happen across the team, in every direction.
How we code review
As an agile software company, Queue-it practices and attains positive results out of our informal code reviewing, which is incorporated into our lean/agile software development process as one step.
When a developer has finished a small task and its unit tests, and submitted it into code our repository, another developer will then review the code. The original developer will send a message containing the task and its change-set ID (to be used to by reviewer for retrieving new changes from the repository). The reviewer will do a code inspection to see if the code has bugs, if it follows coding conventions and software best practices, etc. A functional test will then be performed (doing integration or system testing). If the code has a bug or if the reviewer has comments (which commonly happens), the user story will be placed back at the previous step in our Kanban board (under development), and this process is repeated until the code is perfected. This process is informal and can be done by messaging, e-mailing, Skype calling, or sitting together at one computer and discussing.
With bigger tasks, developers shelve their code (that is usually not completed or fully unit tested) and ask for reviews. Shelving is a Microsoft TFS practice where you save changes into the code repository server without checking the new code in, so codes can be shared/accessed between developers. Therefore, architectural/design code reviews can be made by other developers without breaking the build and before actually committing the new code. After the primary review (once, in our case) is done and when the code is completed, the same workflow as what was described in our small task process occurs.
Code review best practices
Queue-it code review best practices include defining short user stories, generating short code review tasks (in terms of time), combining code review with functional testing, and performing non-blocking code reviews.
- The key way we perform painless code reviews is to break them into short users stories and tasks, which means that having only a few lines of code changes for each story results in a short review time (usually between 15 minutes to 1 hour for normal tasks, and up to 3 hours for bigger tasks).
- We consolidate functional testing with review tasks in our short stories, so the reviewer does both a code inspection and makes sure that the functional requirement of the user story is satisfied. Combining these saves resources, as the reviewer spends time with the user story and understands its requirements, and is, therefore, able to perform an insightful functional test as well as a code review.
- Our review tasks usually are non-blocking, so developers are free to commit their changes (we do post-commit reviews for small stories) and then ask another developer to review their codes while they continue with their other user stories. Generally, we do not get many tasks waiting for reviews because reviewing does not take a long time, so small code review tasks happen in one day and the owner of the story can continue with code review results in the next day.
What we look for in code review
Generally, there are different factors to consider when performing code inspections, including coding style, comments, and finding defects. Error-prone parts of code is, therefore, a good place to start, including control structures and logical rich parts of code.
We also look for how the code is handling multithreading and performance. The other factor is code quality: Does it follow coding best practices and patterns (like SOLID etc.), and our internal coding convention (is it consistent with our code base)? Have code changes been unit tested, had exception handling and logging (if any), etc.?
We would also recommend reading the many useful review guidelines and review checklist articles published that you can get inspiration from.
Code review benefits
Code reviewing forces us to think twice about our code structures and reflect on our development styles. Usually our review comments are about class designs and code structures: “Wouldn’t it be better if you put this part of the code in another class?” or “How about making a separate class and injecting that into this class?” or “Why did you inherit from this class and not just create a new one?”, etc.
Yes, names and comments are important (even for an agile team). We want to share readable code that we can eventually share, so our reviewers focus on how understandable and precise our code is.
Code reviews provide non-stop learning and training, no matter your coding level, and lets developers share new features (of the language, framework, or tools, etc.) and technical tricks that you may have not seen or used before. You can improve your skills and repertoire by code reviewing with your peers.
We share knowledge about our individual tasks with our teammates so that everyone distributes responsibility and specialties equally, which means we can always cover other tasks in case someone is on holiday.
We have used code reviewing to make new developers familiar with our system and also mentor them with it.
Our simple rule of “reviewing code before deploying it” has had a successful outcome in our team. The next step for us would be to incorporate third-party tools and review templates into our code reviewing to make the review processes more structured while keeping our review practice lightweight.
For more on code review, there are many good books and articles about the importance, effectiveness, applicability, and best practices of code reviewing as a part of software development process, including ‘Peer Reviews in Software: A Practical Guide’ by Karl Wiegers.