The problem is… most developers hate code reviews and avoid them like the plague. If they can’t avoid them, then they show up and act their part like in a play at the theater of the absurd. They fight getting asleep during the formal inspections and try to make them pass as fast as possible by not commenting much on anything.
The sad part is, most of the time, they cannot comment because they have no clue how the inspected code works. They were not involved in the design of that code and they didn’t have any time to prepare for the inspection. Out of boredom or maybe to show some level of awareness they sometimes cause long discussions about the spelling mistakes in comments and the names of the variables. This consumes most of the allocated time for the review and as a result in the last few minutes everybody rushes to go through 90% of the code just to avoid another meeting. As a result the most important bugs go unnoticed through the code reviews right into the shipped code.
The supporters of the traditional code reviews (Fagan style) always defend the method and place the guilt on the teams who don’t properly implement code reviews. I don’t buy this but the question remains: if everybody thinks code inspections are useful, then why aren’t they done properly? Is it because properly done code reviews are prohibitively expensive? Let’s see…
The traditional perceived value of Fagan style code reviews, the idea that they have as main purpose finding bugs in the code, is one of the big problems of the methodology. Code inspections or reviews as we know them come from the hardware world. Initially Michael Fagan used them for hardware design reviews. This fact is very significant because uncaught defects in hardware design are pretty expensive.
Also defects in low level software were very expensive at the time. Imagine you write a piece of code that has to be burned on a ROM chip (non erasable). You cannot afford to wait and test/debug your code until after the ROM was destroyed. But this is not the case anymore. Unit tests and running the code in debuggers for success and failure paths are pretty cheap with modern IDEs and testing frameworks.
Done properly code reviews are extremely time consuming (take the conclusions of that link with a grain of salt since they sell tools for code reviews). Today’s programmers are more efficient, due to tools and better languages, then 30 years ago. They can create more code faster. But the amount of code inspected per hour is about the same because they are not smarter than people 30 years ago. Imagine 1000 lines of code that one programmer can write in 1-2 days.
With a traditional code inspection 4 people are required to inspect this code at a rate of 200 LOC/hour after they prepared the code review by reading the code and understanding the design. If we are optimistic the whole thing will require 6-8 hours per developer. This means a total of 24-32 hours spent on inspecting code produced in 8-16 hours. Probably this kind of review can find a lot of bugs in the code but is it worth the price? How many teams get away with estimates including 2-3 hours of inspection for every hour of development?
I don’t doubt there are exceptions to this scenario. There are certain situations where any other way of finding bugs is way too expensive. When we send software on Mars we better inspect every line of that code twice because it is too expensive not to. But in most software shops around the world this is not the case. Code reviews are not done by the book because it is impossible within the constraints of most projects.
As a result of having no chance of doing them properly but being forced by the process to do them, people start to pretend doing them:
- Code reviews become process bureaucracy and they are treated as such by developers
- Since they cannot prepare for code reviews because of lack of time people have no interest in the inspected code
- Everybody is busy with their own assignments and it becomes pretty difficult to find at the same time 3-4 programmers that have the time and the design knowledge to inspect a specific piece of code. The developers that have the time (if any) might have not been involved in the design of that piece of code.
- People take it as a chore sice they are not judged by the quality of the inspection and how many bugs they find in the inspected code. Their are evaluated by the code they produce and code reviews eat in their coding time.
- The lack of enough knowledge about the inspected code leads to a meaningless ritual that concentrates on form not fundamentals. This is why people talk about code formating and comments in code inspections.
- Also this lack of knowledge leads to meaningless metrics collected in the review documents and used by the management to prove the effectiveness of code inspections.
Read page 2: Changing the focus of the code review
Pages: 1 2