Attention! This article is meant for those people, who have an idea of what a code review is and who want to implement this technology in their companies.
When we started implementing code reviews in our projects, we were disappointed by the lack of good materials related to the process organizing from the very beginning. One more aspect that has hardly ever been described is review scaling.
Filling this gap in, we want to share our experience in implementing this wonderful practice by our team. Constructive comments are welcome.
So let’s get it started.
What is it for?
First of all let’s define goals we want to achieve reviewing a code. Of course these goals differ in case of each project and project team. They are influenced by a project character (either one-time or long-term), lifetime (short or long maintenance cycle), and etc. The following goals are the most important to us:
- Decreasing number of defects, detected by our colleagues from the software quality control department and by company’s clients.
- Reducing an application maintenance cost due to increase of code quality.
- Securing quality and quantity of unit tests.
- Securing joint code ownership.
- Securing experience interchange among team members.
- Perfecting a code style. Detecting and discussing style controversies within the team.
Who participates in a review?
Let’s define several terms that will be used within the topic.
- Author is a code developer.
- Reviewer is a developer responsible for all changes getting into a particular module or a path in a project branch.
- Observer is a developer employed as an expert.
When to review?
Now let’s define a place of code reviews in the development process, time of reviews: either before adding a code to a repository (pre-commit) or after adding (post-commit). The choice should be made very carefully, because implementation of code reviews is often quite delicate. Those teams, in which private code ownership prevails (and it happens pretty often), are exposed to the risk most of all. That is why it is reasonable to implement post-commit reviews at first to minimize the risk of failure to meet the project deadlines due to inevitable “holy wars” so common in the beginning. As the participants of a project team gather necessary experience, pre-commit reviews can be implemented.
How does it work?
A developer, creating a review, adds the following participants:
- a reviewer of their group;
- a lead of their group.
The group lead assigns observers from the number of group leads, which modules have been changed.
The group leads assign reviewers from their groups.
What is needed for implementation?
Several terms should be complied with to implement code reviews successfully.
- Before a code is added to a repository, it is surely reviewed by at least one person, who knows it well.
- Developers always know about any changes introduced into their projects by other groups.
- A group lead knows everything what the group does and gets a good overview of any code of the group.
- Within a group, developers have sufficient knowledge of a code written by their colleagues.
- If these terms are complied with, project participants achieve a good level of collective code ownership.
This is it, I think :)
If the IT community is interested in the topic of a code review and description of our experience, then we’ll dedicate one of our next articles to automation of reviews using SmartBear’s CodeCollaborator.
Code reviews in most organizations are a painful experience for everyone involved. The developer often feels like it’s a bashing session designed to beat out their will. The development leads are often confused as to what is important to point out and what isn’t. And other developers that may be involved often use this as a chance to show how much better they can be by pointing out possible issues in someone else’s code.
Code reviews, however, don’t have to be painful.
Remembering the Purpose
Code reviews have two purposes. Their first purpose is to make sure that the code that is being produced has sufficient quality to be released. In other words, it’s the acid test for whether the code should be promoted to the next step in the process. Code reviews are very effective at finding errors of all types, including those caused by poor structure, those that don’t match business process, and also those simple omissions. That’s why they are an effective litmus test for the quality of the code.
The second purpose is as a teaching tool to help developers learn when and how to apply techniques to improve code quality, consistency, and maintainability. Through thoughtfully evaluating code on a recurring basis, developers have the opportunity to learn different and potentially better ways of coding.
Code reviews often start off on the wrong foot because they are perceived as an unnecessary step that has been forced upon the developers or, in some cases, evidence that management doesn’t trust the developers. Neither of these perspectives is accurate. Code reviews are a proven, effective way to minimize defects. Whatever additional motivations the organization has for performing code reviews, they are, at their core, an industry best practice.
A Matter of Approach
One of the other ways that a code review gets off track is by the participants approaching it as though the process is designed to demonstrate who the better programmer is. Code reviews often become mental jousting matches where people take shots at a target; in other words, the developer that wrote the code being reviewed. A better approach is a learning approach where the whole exercise is viewed as a forum to discuss and learn from everyone.
Saying that the approach should be educational and open is one thing but creating that feeling when the history has been mental jousting matches can be quite challenging. That being said, there are a few simple things that you can do to change the approach for the better:
- Ask questions rather than make statements. A statement is accusatory. “You didn’t follow the standard here” is an attack—whether intentional or not. The question, “What was the reasoning behind the approached you used?” is seeking more information. Obviously, that question can’t be said with a sarcastic or condescending tone; but, done correctly, it can often open the developer up to stating their thinking and then asking if there was a better way.
- Avoid the “Why” questions. Although extremely difficult at times, avoiding the”Why” questions can substantially improve the mood. Just as a statement is accusatory—so is a why question. Most “Why” questions can be reworded to a question that doesn’t include the word “Why” and the results can be dramatic. For example, “Why didn’t you follow the standards here…” versus “What was the reasoning behind the deviation from the standards here…”
- Remember to praise. The purposes of code reviews are not focused at telling developers how they can improve, and not necessarily that they did a good job. Human nature is such that we want and need to be acknowledged for our successes, not just shown our faults. Because development is necessarily a creative work that developers pour their soul into, it often can be close to their hearts. This makes the need for praise even more critical.
- Make sure you have good coding standards to reference. Code reviews find their foundation in the coding standards of the organization. Coding standards are supposed to be the shared agreement that the developers have with one another to produce quality, maintainable code. If you’re discussing an item that isn’t in your coding standards, you have some work to do to get the item in the coding standards. You should regularly ask yourself whether the item being discussed is in your coding standards.
- Make sure the discussion stays focused on the code and not the coder. Staying focused on the code helps keep the process from becoming personal. You’re not interested in saying the person is a bad person. Instead, you’re looking to generate the best quality code possible.
- Remember that there is often more than one way to approach a solution. Although the developer might have coded something differently from how you would have, it isn’t necessarily wrong. The goal is quality, maintainable code. If it meets those goals and follows the coding standards, that’s all you can ask for.
What to Do If You’re a Developer
The above advice is fine if you’re the project or development leader who is organizing the code review, but what if you’re the one who has to endure a painful code review? What can you do to make the process less painful if you’re the developer who’s having your code reviewed?
- Remember that the code isn’t you. Development is a creative process. It’s normal to get attached to your code. However, the folks who are reviewing the code generally aren’t trying to say that you’re a bad developer (or person) by pointing out something that you missed, or a better way of handling things. They’re doing what they’re supposed to be doing by pointing out better ways. Even if they’re doing a bad job of conveying it, it’s your responsibility to hear past the attacking comments and focus on the learning that you can get out of the process. You need to strive to not get defensive.
- Create a checklist for yourself of the things that the code reviews tend to focus on. Some of this checklist should be easy to put together. It should follow the outline of the coding standards document. Because it’s your checklist, you can focus on the thing that you struggle with and skip the things that you rarely, if ever, have a problem with. Run through your code with the checklist and fix whatever you find. Not only will you reduce the number of things that the team finds, you’ll reduce the time to complete the code review meeting—and everyone will be happy to spend less time in the review.
- Help to maintain the coding standards. Offer to add to the coding standards for things discussed that aren’t in the coding standards. One of the challenges that a developer has in an organization with combative code review practices is that they frequently don’t know where the next problem will come from. If you document each issue into the coding standards, you can check for it with your checklist the next time you come up for code reviews. It also will help cement the concept into your mind so that you’re less likely to miss opportunities to use the feedback.
When Code Reviews Aren’t Face-to-Face
Sure the above techniques can help you when you’re sitting across the table from someone but how do you communicate tone and approach when you’re doing code reviews through marking up the developer’s code. Surprisingly, it’s often easier than face-to-face code reviews.
In a face-to-face code review you, have to think on your feet. You can’t reread your statements and questions and then change them to be more sensitive, more precise, or more caring. The benefit of an off-line code review is most heightened at the beginning of the process. Both the development leader and the developer have the opportunity to think about how to respond.
By the way, this off-line review process is the one that books (and some articles use.) This off-line process is used all the time in the publishing world, but is strangely not frequently used for code reviews. In some ways, it can be much more effective than a face-to-face conversation. Even if you have the ability to do meetings, you may consider moving to an off-line review to make the process better.
If you do your reviews by comments, a few special techniques are called for. They are:
- Put a summary comment at the top—and be positive. One of the beautiful things about a review that is done off-line is that it’s possible to read through everything and make a summary statement at the top of the file. This can help set the mood of the developer so they understand whether you’re thrilled, thankful, happy, and so forth. This will soften comments even if harsh at times and make them more palatable. No matter what, the initial comment should contain some element of a positive message because there’s something good to say about nearly every piece of code—even if it is only that it compiles. (It is syntactically correct.) Setting that positive attitude is essential.
- Use an electronic mechanism to record the comments. Whether you export the code to a PDF file and use comments to mark it up, or copy the code into Word and use Word’s commenting features, make sure that you use an electronic format for comments. This is important because it doesn’t limit you to how much you can fit on a page—so you can explain more of what you mean and you can be more questioning and caring than doing a hardcopy review allows.
- Make an upfront agreement that not every question needs to be responded to. The best off-line reviews include thinking questions. Questions such as “Would it be better to implement a provider pattern here?” don’t necessarily need a response. Make it clear to the developer that some comments are simply to provoke thinking. This allows you to identify areas where you want to make sure the developer is considering alternatives without raising them to the point of being an issue with the code. Getting developers to think about the code they are writing doesn’t improve the quality of the code they’ve already written, but it does have a long-term positive impact on code quality.
Code reviews are often misused and painful for everyone, but they don’t have to be. Some simple steps can convert torture into teaching and improve the long-term outlook for code quality in your organization.
Peer review — an activity in which people other than the author of a software deliverable examine it for defects and improvement opportunities — is one of the most powerful software quality tools available. Peer review methods include inspections, walkthroughs, peer deskchecks, and other similar activities. After experiencing the benefits of peer reviews for nearly fifteen years, I would never work in a team that did not perform them.
After participating in code reviews for a while here at Vertigo, I believe that peer code reviews are the single biggest thing you can do to improve your code. If you’re not doing code reviews right now with another developer, you’re missing a lot of bugs in your code and cheating yourself out of some key professional development opportunities. As far as I’m concerned, my code isn’t done until I’ve gone over it with a fellow developer.
But don’t take my word for it. McConnell provides plenty of evidence for the efficacy of code reviews in Code Complete:
.. software testing alone has limited effectiveness — the average defect detection rate is only 25 percent for unit testing, 35 percent for function testing, and 45 percent for integration testing. In contrast, the average effectiveness of design and code inspections are 55 and 60 percent. Case studies of review results have been impressive:
- In a software-maintenance organization, 55 percent of one-line maintenance changes were in error before code reviews were introduced. After reviews were introduced, only 2 percent of the changes were in error. When all changes were considered, 95 percent were correct the first time after reviews were introduced. Before reviews were introduced, under 20 percent were correct the first time.
- In a group of 11 programs developed by the same group of people, the first 5 were developed without reviews. The remaining 6 were developed with reviews. After all the programs were released to production, the first 5 had an average of 4.5 errors per 100 lines of code. The 6 that had been inspected had an average of only 0.82 errors per 100. Reviews cut the errors by over 80 percent.
- The Aetna Insurance Company found 82 percent of the errors in a program by using inspections and was able to decrease its development resources by 20 percent.
- IBM’s 500,000 line Orbit project used 11 levels of inspections. It was delivered early and had only about 1 percent of the errors that would normally be expected.
- A study of an organization at AT&T with more than 200 people reported a 14 percent increase in productivity and a 90 percent decrease in defects after the organization introduced reviews.
- Jet Propulsion Laboratories estimates that it saves about $25,000 per inspection by finding and fixing defects at an early stage.
The only hurdle to a code review is finding a developer you respect to do it, and making the time to perform the review. Once you get started, I think you’ll quickly find that every minute you spend in a code review is paid back tenfold.
If your organization is new to code reviews, I highly recommend Karl’s book, Peer Reviews in Software: A Practical Guide.
- $ 50 Million down the drain
- 'Call of Duty' top U.S. monthly sales
- – OPRO Japan Announces the Launch of OPROARTS Apps Easy Merge PDF for Salesforce.com
- 5 Regular Expressions Every Web Programmer Should Know
- 50 Common Web Design Mistakes
- A Collection of Linux Commands
- A comparison between the Google's GWT (Google Web Toolk
- A Future With Nowhere to Hide?
- A innovative mouse pad
- A pioneer who laid the path for many to tread on
- A Used Computer Can Change a Whole Village – History of
- Advantages of changing your job
- AIDS Threat for BPO Romeos
- Aitken Spence ventures into IT with EVES IT Lanka
- AJAX: The veins of Web 2.0
- All Things Bloggable by Brian and Stephanie ReindelThe
- An evening with Google's Marissa Mayer
- and NetBeans IDE
- Anish Malhotra – Cloud Computing – interop Mumbai 2009
- Annoying NetBeans 6.0 Facelets Support Issue on Tomcat
- “Highly Customizable PDF Sales Document Generation Solution”
- “Totally Free Word Mail Merging Solution” – OPRO Japan Announces the Launch of OPROARTS Apps Easy Merge for Salesforce.com
- ගම්දනව් කරා තොරතුරු තාක්ෂණය ගෙන යන්න පරිගණක විද්යාගාර 1000ක්
- ජංගම දුරකතනයෙන් පුවත් ආක්රමණයක්
- Become a young Project Manager after A/Ls
- Beyond Preferences API Basics
- Beyond Struts – The Way Forward
- Bloggers then and now?
- BPO in the A’pura backwoods
- BPOs feel realty pinch in smaller cities
- Business Process Outsourcing – 'Other side of the white
- Can JSF speed up Web application development?
- Care your eyes
- Click on to Lanka’s bloggers
- Cloud computing – for whom?
- Code Smart: Solution to classpath too long (aka input l
- Colombo among top Tech outsourcing cities
- Common Web Design Mistakes Prevent Google From Indexing
- Comparison of OpenDocument and Office Open XML formats
- Computer gamers 'have reactions of pilots but bodies of chain smokers'
- Computer viruses hit one million
- Consumer memo: Google tests health record service
- Creating a common lexicon for software development in y
- Design Patterns Quick Reference
- Dialog launches mobile waste recycle programme
- Dilbert Cartoons : Marketing and Sales
- Don't Use Mobile Phone During Storms
- Dynamic PDF generation with JasperReports
- E-government policy reduces public inconvenience
- E-mail is ruining my life!
- e-NIC’s another flop
- Ecma Office Open XML document format now ISO/IEC Standa
- Effective Writing Skills Presentation
- ESB links
- Essential firefox add-ons for web programmers
- Export Swing components to PDF
- Facebook puts into place new safety controls
- Facebook targets Google with program for other social n
- Facebook: A communication tool for IT resellers
- Fighting the OutOfMemoryError
- Five Things Web Developers Should Stop Doing
- Flex and Java – A perfect technological marriage
- Flexible reporting with JasperReports and iBATIS
- Focusing on Problems vs. Focusing on Solutions
- Gadgets 'threaten energy savings'
- Gadgets threaten energy savings’77
- Generating Huge reports in JasperReports
- Gmail in Sri Lanka among those hacked
- Good advice for creating XML
- Google Analytics collaborates with Roche Diagnostics Co
- Google unveils personal medical record service
- Google Wave: Ripple or Tsunami for Research
- Hewitt study reveals key trends in salary increases acr
- Hibernate dynamic mapping and Dom4J enabled sessions
- How to call Stored Procedures from JasperReports
- How to Make a JAR executable in Windows?
- HowTo: JasperReports framework. Deployment to Tomcat
- implications and practices
- Indian paraplegic becomes stock market winner
- Integrating and Using JasperReports in NetBeans
- IPTV- Sri Lanka
- IT companies woo women
- IT going green
- IT services market 'will shrink for two years'
- Japanese Software
- Java Performance Tuning
- Java problem — Too many open files
- JavaServer Faces (JSF) vs Struts
- JavaSpecialists Issue 146 – The Secrets of Concurrency
- Job Scheduling in Java
- Job scheduling with Quartz
- Leadership Lessons from a One Year Old
- Life of a software engineer
- Liferay and Pentaho Integrated
- Liferay open source portal adds content management
- Linux Operating Systems market grows
- LTTE a trend setter in cyber terrorism
- make IT simple — First You Have To Make IT Manageable
- Make Utorrent FASTER! (1.8.2 UPDATE)
- Most popular free/open source IDEs and Editors
- Move to create less clumsy robots
- MOZILLA FIREFOX CHEAT SHEET
- My Favorite Firefox Extensions
- MySpace and Google join to launch new social platform
- Napkin Look and Feel
- Native file locks in java
- NetBeans 6 M7 released. There are some cool features av
- NEW OFFICE POLICY
- Nowhere To Hide: The Battle For Fallujah
- One chip does three memory jobs
- One To Many MappedBy
- Open Source Java Reporting with JasperReports and iRepo
- Open Source Software in Java(tm) Links
- Playstation By The Numbers
- Preparing for renewed growth: Setting strategy for IT and the business
- Private Facebook Pages Are Not So Private
- Programming and Managing : Can you do both?
- Programming Can Ruin Your Life
- Programming Handbooks and Cheatsheets
- Project Management for the 21st Century
- Rajiv murder accused now Master’s in Computers
- Raman Roy upbeat on Lanka as hub for financial BPO
- Reinventing the Foot Soldier
- Reporting Made Easy with JasperReports and Hibernate
- Researchers Find Way to Steal Encrypted Data
- Resources for the professional web designer
- SaaS 101: What Managers Need to Know
- SaaS Accelerates Despite Economy’s Woes
- SaaS for Rough Economic Times
- SaaS Model Economics 101a – Aggregating Customers for Low Cost Advantage
- Scheduling Jobs in a Java Web Application
- Secrets Of The Masters: Core Java Job Interview Questio
- Secuirty problem casts doubt on E-passports
- Security and social networking
- Serious security flaw found in IE
- Single Sign On
- Sinhala / Tamil News Alerts a first in Sri Lanka
- Six High-Tech Survival Strategies in Tough Times
- Small PCs big news as economy slows
- Social Media Optimization
- Social networking applications can pose security risks
- Software as a Service (SaaS) Adoption
- Software Developers are Born Brave
- Software Piracy is Helps the planet to green
- Spring on Code organization for large projects
- Sri Kanth walks tall in the IT world
- Sri Lanka ranks no.2 of the software piracy in Asia
- Sri Lanka unable to meet targeted workforce in the IT s
- Sri Lanka under fire over Internet censorship
- Sri Lanka's youth programmers battle for Microsoft reco
- Sri Lankan names cause Italian computers to break down
- Struts and a database
- Success of Google – Running the business in Buddhist wa
- Success Tips to Project Managers
- Swing-based tree layouts with CheckboxTree
- Tech stocks for tough times
- Techno Updates
- Technology for Country Folk
- TECHNOLOGY-SRI LANKA: Leapfrogging Out of Poverty on I
- Ten things every Java developer should know about UNIX
- terrorists' behaviour pattern on Internet
- The 25 Best High-Tech Pranks
- The 4 Unlikely Traits of Good Developers
- The fable of the Cracked Water Pot
- The Future of Shopping
- The Latest Java Innovation : JavaFX
- The PaaS Era : Everybody's Pounding Out Mashups
- The Real True Value of Software as a Service (SaaS)
- The Ultimate Remote Control
- Thousands of convicted sex offenders evicted from MySpa
- To all Sri Lankan on working late.
- Top 10 Eclipse Hotkeys
- Top 10 Firefox Business Extensions
- Top 10+ source code search engines
- UI design with Tiles and Struts
- UK software
- USAID Partners with Microsoft to Unleash Lanka’s ‘U
- Useful windows RUN Commands …
- Using Enhanced For-Loops with Your Classes
- Using Form Based Authentication
- Using Hibernate queries with JasperReports
- Using Jasper Reports with Visual Web Pack
- Virtually Safe: Five real-life pitfalls of online worlds — and some safer alternatives
- Virtusa Corp. lowers revenue guidance
- Virtusa partners with academic institutes to expand IT
- Virtusa ventures into social media
- Warn Doctors
- Wasted time or ego boost?
- Web 2.0 – The power shift
- What if you're the SaaS?
- What is Defensive Copying
- What is JiBX?
- What Is Quartz
- what is Software as a Service (“SaaS”)
- What Is Web 2.0
- WHY EMPLOYEES LEAVE ORGANISATIONS ?
- Wikipedia: Our virtual Middle Ages
- Wireless in the World
- Wisdom of the masses
- Women and IT – Capturing YouTube Videos
- World's fastest supercomputers (Blue gene/L)
- Your Next Computer