New to the concept of code review? This post explains what code review is and why information technology's of import.

Disclaimer: The following document is heavily based on the Mozilla Code Review FAQ [1][2]. Only my team at VAIRIX has made many adaptations in order to reverberate the two-level review procedure that is part of our evolution methodology.

What is lawmaking review?

As Wikipedia puts information technology, "Code review is systematic examination … of figurer source code. Information technology is intended to find and ready mistakes overlooked in the initial development stage, improving both the overall quality of software and the developers' skills."

What is the purpose of code review?

Code review is the well-nigh commonly used procedure for validating the design and implementation of features. It helps developers to maintain consistency between pattern and implementation "styles" across many  team members and between various projects on which the company is working.

We perform code review in ii levels. The first is known as peer review and the 2nd is external review.

The code review procedure doesn't brainstorm working instantaneously (especially with external review), and our process is far from being perfect — although we take done some serious research around the topic [3]. So, we are always open up to suggestions for comeback. :)

Having said that, let's dig into peer reviews.

Code Review - VAIRIX Agile Code Review

What is a peer review?

A peer review is mainly focused on functionality, pattern, and the implementation and usefulness of proposed fixes for stated problems.

The peer reviewer should be someone with business knowledge in the trouble area. Likewise, he or she may use other areas of expertise to brand comments or suggest possible improvements.

In our company, this is necessary because we don't practice blueprint reviews prior to code reviews. Instead, nosotros expect developers to talk to each other about their design intentions and get feedback throughout the (usually non-linear) design/implementation process.

Accordingly, we don't put limitations on what comments a reviewer might brand about the reviewed code.

What do peer reviewers await for?

Feature Completion

The reviewer will make sure that the code meets the requirements, pointing out if something has been left out or has been done without asking the client.

Potential Side Furnishings

The reviewer volition cheque to see whether the changed code causes any bug in other features.

Readability and Maintenance

The reviewer volition make sure the code is readable and is non as well complicated for someone completely new to the project. Model and variable names should be immediately obvious (again, even to new developers) and equally short as possible without using abbreviations.

Consistency

Conducting peer reviews is the best approach for achieving consistency across all visitor projects. Define a lawmaking style with the squad and so stick to it.

Performance

The reviewer will assess whether code that will be executed more than often (or the most disquisitional functionalities) tin can be optimized.

Exception Handling

The reviewer volition make sure bad inputs and exceptions are handled in the way that was pre-defined past the team (it must be visible/accessible to everyone).

Simplicity

The reviewer will appraise whether there are whatsoever simpler or more elegant alternatives bachelor.

Reuse of Existing Code

The reviewer will bank check to see if the functionality can be implemented using some of the existing code. Code has to exist aggressively "DRYed" (as in, Don't Repeat Yourself) during development.

Test Cases

Finally, the reviewer will ensure the presence of enough test cases to get through all the possible execution paths. All tests have to pass before the code tin be merged into the shared repository.

Code Review - Peer Review

What is an external review?

An external review  addresses dissimilar problems than peer reviews. Specifically, external reviews  focus on how to increment code quality, promote best practices, and remove " code smells."

This level of review will look at the quality of the code itself, its potential furnishings on other areas of the project, and its adherence with company coding guidelines.

Although external reviewers may  non take domain expertise, they practice  take discretion to raise red flags related to both the blueprint and code  and to suggest ways to solve problems and refactor lawmaking as necessary.

What do external reviewers look for?

Readability and Maintenance

Like to to a higher place, the reviewer will make sure the code is readable and is non also complicated for someone completely new. Once more, all model and variable names have to exist immediately obvious (fifty-fifty to new developers) and as brusque as possible without using abbreviations.

Coding Fashion

The reviewer will ensure that anybody adheres to a strict coding style and volition use lawmaking editors' congenital-in helpers to format the code.

Code Smells

Finally, the reviewer will keep an eye out (or should that be a nose out?) for code smells and make suggestions for how to avoid them.

In case the term is new to you, a code smell is "a hint that something has gone wrong somewhere in your lawmaking. Utilize the smell to track down the problem."

Must external reviewers be "domain experts"?

External reviewers don't have to have domain cognition of the code that they will be reviewing. [iv].

If they know about the domain, they will feel tempted to review it at a functional level, which could lead to burnout. Withal, if they take some business concern noesis, they can estimate more easily how complex the review  will be and can quickly complete the review, providing a more comprehensive evaluation of the code .

So, domain expertise is a bonus, non a requirement.

20150819_162034 (1)

What if an external reviewer misses something?

Nosotros do not expect an external reviewer to brand everything perfect. Something will about likely be missed . The external reviewer does not become responsible for the developer'due south work by reviewing information technology.

How fast should developers receive a response from the external reviewer?

If a programmer has requested an external review , he can look some type of response inside two hours. At the very least, the response should tell him a timeframe for completion.

In some cases, the external reviewers might not respond. They're not perfect and might have too much work to do. Developers should feel free to ping them once again if they don't hear back within 2 hours or attempt with another external reviewer.

Why tin't developers just merge their code into the main branch at present and ask for an external review later?

At that place are many reasons this is a bad idea, but here are 2 of the most important:

  1. External reviews catch problems that would touch on everyone if the lawmaking were merged into the main repository. It doesn't make sense to crusade anybody to suffer for issues that could have been caught by an external review .
  2. The process of merging lawmaking causes the programmer to feel that the work is done, and it'due south time to proceed to the adjacent thing. It'south silly to accept people feeling like something is checked off the task list when it's really not.

Can the external reviewer ask the developer to do something that is not precisely related to the lawmaking?

Yes, the external reviewer has some discretion hither.

We don't think that continuously  making auxiliary changes that are unrelated to the core functionality is the correct thing to do on reviews. On the other manus, small changes (or changes that aid the code maintain a consequent style) may be requested.

There should be a reasonable relationship betwixt the telescopic of the developed functionality and the scope of the requested change.

References

[one] Knous, M. & Dbaron, A. (2005). Code Review FAQ. Mozilla Development Network. Retrieved from https://developer. mozilla .org/en/docs/Code_Review_FAQ.

[2] Rigby, C., German language, D. (2006). "A preliminary test of code review processes in open source projects." University of Victoria Technical Report: DCS-305-IR. Retrieved from http://ifipwg213.org/system/files/Rigby2006TR.pdf.

[three] Macchi, D., & Solari, M. (2012). Software inspection adoption: A mapping study . In Conferencia Latinoamericana de Informática (CLEI 2012). http://ieeexplore.ieee.org/xpl/articleDetails.jsp?arnumber=6427197.

[4] Mozilla (2012). Retrieved from http://www.mozilla.org/hacking/reviewers.html.

Feature epitome credit. All other images were provided by the writer.

Gear up to start your projection?

Learn how ThinkApps tin get your product launched faster, better, and with more value than you knew was possible.

About the author

Darío Macchi

Darío Macchi

Darío Macchi is the COO/Scrum Master of VAIRIX Software Development, a bazaar Ruby-red on Rails development visitor from Uruguay. Likewise he is a Software Engineering Professor, researcher (MSc, starting his PhD) and independent advisor/consultant. He believes in the usefulness of bringing tools from scientific research environments to daily activities of software development companies.

Gratuitous this weekend: Honor winning book,"How to First a Startup" on Amazon kindle. Snag your gratuitous copy today! https://t.co/YmEkcYKp3l

FREE this weekend: Award winning book,"How to First a Startup" on Amazon kindle. Snag your costless re-create today! https://t.co/YmEkcYKp3l

Our new volume, "How to Showtime a Startup," is the ultimate reference guide on tech startups. https://t.co/YmEkcYKp3l