Code reviews? - Page 4

Do you have a question? Post it now! No Registration Necessary

Translate This Thread From English to

Threaded View
Re: Code reviews?

Quoted text here. Click to load it

Point well taken.  What I was getting at with my comment about
3 or 4 being ideal was the undesirable "one developer, five
reviewers" situation.  3-7 would work well with the division of
labor you list.

Needless to say, if there are seven people in the room, any
splitting off into side conversations must be suppressed
with an iron fist.

Re: Code reviews?

Quoted text here. Click to load it
Post you replied to: Tue, 23 Nov 2004 23:18:24 +0000


Re: Code reviews?

"Guy Macon" < skrev i meddelandet
Quoted text here. Click to load it
Sure, if you don't know what you are doing you always have a good chance of

What I question is the value of the code reviews (given that you know how to
do it properly). Code reviews will improve the quality of the code, no
question about that, and most projects should have them. But how much will
they improve the code? Not that much I say.
In many of the projects I have worked in it would have been better to spend
less time on code reviews and more time on design and testing. These
projects did have a good design and good test cases but still I think too
much time was spent on code reviews.

Re: Code reviews?
Quoted text here. Click to load it

My experience is completely opposite. The reason, I think, is because
we have reused a lot of code, and same code again and again for several

Quoted text here. Click to load it

Obviously I have no clue how much time you did spent in code reviews.
But still I have never ever seen a single properly commented code.
And I do not think I will ever see.


Jotkut pitävät armeijaa parhaana järjestelmänä joissakin tapauksissa.
We've slightly trimmed the long signature. Click to see the full one.
Re: Code reviews?
Quoted text here. Click to load it

Rule Number one.  BE SURE what you're reviewing!!!

Horror Story:
Did a project to convert an FPGA to an ASIC.
Did design reviews out the wazoo.  ASIC came out EXACTLY like the spec.
Only problem was that somebody forgot to roll the version at the last
FPGA update.  So the timing mod to compensate for the bad display
drivers didn't get included.  Classic example of GIGO.  $17K down the drain.

Return address is VALID.
500MHz Tek DSOscilloscope TDS540 $2200
We've slightly trimmed the long signature. Click to see the full one.
Re: Code reviews?
Quoted text here. Click to load it

Nice aneckdote ;) I wonder if extensive code reviews leads more
often to the false assumption that everything is covered for.

Thanks, Frank.
(remove 'x' and 'invalid' when replying by email)

Re: Code reviews?

Quoted text here. Click to load it

Nice rule and nice anecdote to go with it.

Each review has to start with a number of cross-checking actions.

Specification issue level and date; Code issue and date (perhaps even time);
What the Configuration Management Record believes to be the current status.
Problem Reports Received and the records. Latest Change Proposals and Work
Instruction Record.

When all (the flowchart on my HIDECS page gives you an idea of my own
process) those items check out with each other correctly then you can begin
to look at the item to be reviewed.

As for the number of reviews:-

 I prefer to resolve the specification first (usually up to 3 iterations)
before I progress to starting the coding. When I do start the coding the
first run through is normally just writing the glossary comment - a sort of
mini-requirement statement - so that I know what I have planned for each
module I am going to build. These glossary comments are then reviewed,
before the code is written, to ensure that the intent for each module is in
conformity to the spec. This might take two iterations at most. When I
start with real code it is tested for intended function by the programmer
almost immediately it is written (remember I use Forth for the most part
and Forth does encourage small functional elements at whatever abstraction
level you are dealing with). There follows a further review which
incorporates a full Fagan style inspection. If this is passed the code is
issued for testing where a formal functional test and a limits test are
performed. The results come back to the review panel who then review the
test results and, if all is OK, approve the code for release to the trusted
library of functions ready to build the rest of the system.

So, from requirements to system integration there would have been 9 or 10
reviews of each element. Once integrated there would be further reviews of
the unit testing, system testing and final integrations testing. As a wrap
up there may also be a formal client witnessed factory acceptance test of
the whole system which would involve yet another review.

While it may seem that there are a lot of reviews the time taken to
accomplish them is of the order of 8% to 10% of the project. The benefit is
that it is quite possible to eliminate at least 95% of all the bugs you are
ever likely to see in the project by taking the issue of proper review and
testing seriously.

It is interesting to note that the longest review meetings actually happen
very early in the project when the requirements and specification documents
are being resolved. These review meetings have occassionally broken the two
hour rule but at that stage it was always worthwhile geting the issues

We've slightly trimmed the long signature. Click to see the full one.
Re: Code reviews?
Write a theory of operations of the code/device. As you do, you will
review and revisit the code to refresh your memory as to why you did
what you did.



martin griffith wrote:

Quoted text here. Click to load it

Site Timeline