TITLE: On the value of code reviews [ As long as we are beginning to talk about "process", I think it will be useful to occasionally include snippets of such discussions from the net. Here we join a discussion in progress on code reviews. One point not mentioned below is the bug detection maxim "the earlier the better". It is cheaper for use to find these bugs earlier in the process. This gives an edge to code reviews over testing since the former tends to occur earlier in the lifecycle than the latter. -adc ] JK = James Kanze US/ESC 60/3/164 #71425 (kanze@us-es.sel.de), 24 Oct 94 LR = larryr@saturn.sdsu.edu (Larry Riedel) LR: I would have thought it was because in many cases it is more efficient and more effective to test the code rather than inspect it. JK: A test can never demonstrate that the code works. It can only demonstrate that it doesn't work. LR: A code review can never demonstrate that the code works. Neither can it demonstrate that the code doesn't work. It can give a pretty good idea one way or the other, but then so can testing. JK: A code review can come significantly closer to demonstrating the correctness of the code, but both code review and testing suffer from one significant disadvantage: the must be done thoroughly to be of any use. How do you determine that the testing is done thoroughly? You review the test procedure. And of course, somewhere, someone is reviewing the review procedures. In the end, the only thing you have to go on is the reviews. For that matter, I could just claim to be writing code, and being paid for it, except that the auditors review my books, and my customers review my code. JK: Testing is useful as a last control, and as a means of keeping the reviews, etc. honest. But all of the studies I've seen confirm my own experience; if your procedure for writing, verifying and reviewing code is correct, very few errors ever get to the test phase. Note too that an error found in code review is easy to correct; you know exactly where the error is. Compare this to finding why the code "core dumped" (because someone misused a pointer hours earlier). LR: Few would argue against the idea that if enough people spent enough time reviewing the code, then eventually almost all of the bugs could be found and removed. Given infinite time and infinite budget, that may be the most thorough and effective approach. Of course testing would have to be performed anyway. JK: According to the studies I've read (which more or less confirm my experience), it takes only a couple of man-hours to review a couple of hundred lines. And such reviews typically find over 95% of the errors. Correcting an error found in review is usually trivial; the review indicates the exact place in the code where the error occurs. (Something along the lines, didn't you forget to initialize this pointer.) Finding why a test core dumped (because an uninitialized pointer modified the vptr in an unrelated class) is generally somewhat more time consuming. My own personal experience would suggest that errors which slip through code review also slip through the tests. There are errors which slip through both code review and tests, of course, but again, my experience would suggest that there are only half as many in reviewed code as in unreviewed code. And of course, no amount of testing will ever be able to confirm the maintainability of the code. A point that I've rarely seen mentioned is that when a programmer knows that his code will be reviewed, he will generally write better code to begin with. If he thinks that no one else will read his code, at least not as long as he is there, there is more of a tendancy to be sloppy and not care, at least in some people. Finally, code reviews are an important part of internal information flow. When code reviews are just starting, this can be the most important part. The less skilled, less experienced programmers learn by reading the code of the more skilled. And the gurus quickly learn to write code that others can understand. (There are two ways the guru can achieve this. He can simplify his code, and he can educate the others to his level. Generally, both ways have overall positive results.) All of which combine to make code review a very cost-efficient means of reducing the number of errors in delivered code. LR: I do not see where the "cost-efficient" part comes in here. Code reviews can find bugs, and testing can too. In some cases testing the code finds bugs at a lower cost (staff hours or lost revenue) than code reviews, and in some cases the converse is true. It cannot be determined a priori which bugs will be found less expensively using which technique, and hence it cannot be determined a priori if using code reviews will cost less. JK: It has not been determined a priori. There have been numerous experiments and a postiori evaluations in this domain, and all that I have seen confirm the cost-efficiency of code review. A code review costs less (in staff hours) that all but the most superficial testing. It finds more errors than intensive testing. And it costs significantly less (about 10 times less is the generally accepted figure) to correct the errors. The first point is mostly offset by the fact that you still want testing, to keep the code reviewer honest. But the really big win is in the last points: you find more errors, and those that you find are easier to correct.