Path: utzoo!utgpu!jarvis.csri.toronto.edu!mailrus!cornell!uw-beaver!apollo!perry From: perry@apollo.COM (Jim Perry) Newsgroups: comp.software-eng Subject: Re: code reviews Message-ID: <43f2d7c0.183dc@apollo.COM> Date: 20 Jun 89 19:26:00 GMT References: <12047@bloom-beacon.MIT.EDU> <116@opel.UUCP> <13716@umn-cs.CS.UMN.EDU> Reply-To: perry@apollo.COM (Jim Perry) Organization: Apollo Division of Hewlett-Packard, Chelmsford, MA Lines: 64 In article <13716@umn-cs.CS.UMN.EDU> meuer@klein.UUCP (Mark Meuer) writes: >In article <116@opel.UUCP> johnk@opel.UUCP (John Kennedy) writes: >> >>I can't help but chuckle when I see code reviews and productivity in the >>same paragraph. It's good for the project, it's what the customer wants, >>but be assured that it's an increase in overhead and a decrease in productivity. >> >>I wish this weren't true. >> >>John > >I beg to differ. I used to work for a software company where peer >reviews were used extensively. They were very helpful in a number of >respects, and were well worth the effort. > >It is true that reviews add overhead to the development, but that time >is made up for by better designs and much better communication and >understanding between members of the programming team. > >-mark I'll second that. Many people seem to think of reviews as either a very formal process where you're going through a lot of unnecessary formality to satisfy some external requirement, or as an antagonistic process with a manager poring over your code like a hostile grader. Clearly neither of these situations are likely to yield a good software engineering environment, which is not to say that there aren't sites out there run that way. My experience of peer reviews is just that: review of code (or a design or whatever) by one or more peers -- engineers whose ability and insights you respect similarly to your own. Just as with writing English, one becomes blind to certain aspects of one's own writing over time, and having a fresh eye look over it can turn up flaws, inconsistencies, etc. It's not an antagonistic process, and you don't expect to turn up lots of problems, but you'd be surprised at the number of things you *can* turn up this way, even in the best code. A very real additional benefit of such an environment: where code is written with the expectation that others will be reading it, the tendency is toward code that is more readable, more understandable, and better documented than otherwise, since it must be understood by a reader who is generally not as familiar with the code as the writer. Since each writer is also a reader, this is to everyone's benefit. Of course all of those aspects are in direct support of future maintainance of such code at very little incremental cost. While I think there's more room for formality in reviewing of design specs and suchlike, code reviews can be quite informal, for instance just having a colleague look over the source/listing of a module, and diffs from the previous release where appropriate, as part of the release/freeze process. It helps if there's a librarian/release coordinator to coordinate the process of generating listings/diffs and allocating reviewers, but even an informal process ("hey, Joe, want to take a look at this?") is better than never having anyone look at your code. It generally doesn't take more than a few minutes to review most coding updates, and even reading a completely new module at this level should take under an hour. (Remember, you trust the writer, and assume it works, you're just double-checking, not grading.) That's pretty minimal overhead, for a pretty big payback. -- Jim Perry perry@apollo.com HP/Apollo, Chelmsford MA This particularly rapid unintelligible patter isn't generally heard and if it is it doesn't matter.