Re: Do you do formal code reviews?

On 11/16/2011 9:47 AM, Robert Newton:

Thanks everyone for their input. I see the majority here are smaller shops
it or one man consulting firms so I can see why there are not any code
reviews needed. Our needs are a little different however.

We are implementing coding standards in our shop and will be enforcing
those standards with regular code reviews. Our shop has 30+ RPG
developers, each with their own styles and techniques varying from
techniques considered old 15 years ago to some guys counting the days
until 7.1 gets installed to get DB2 ALIAS in externally described data
structures.

We have ever expanding presentation technologies that our business logic
in RPG must be able to support. If we do not have code reviews, we will
continue to see business logic tied up in interactive programs and not
pulled out to service programs for reuse from other interactive
applications, web pages, web services, stored procedures, desktop
applications, etc...

Modernizing a shop this size without code reviews and shop standards for
techniques and syntax styling would be impossible (as we have shown over
the years).

Here is one recent study of code reviews:
http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/

Note, however, that the object of these code reviews was to find
defects, not to enforce standards. After more than three decades
programming in the midrange arena, I have never once seen better
applications emerge as a result of a uniform corporate white space and
naming standard.

If I have a poor programmer on my staff (let's say he uses coding style
that's 15 years outdated), why does he code that way? Is it because he
knows better but chooses to be poor? That seems unlikely, doesn't it?
If we presume then that he doesn't know any better, how will imposing
stylistic changes force him to know better vs simply LOOKING like he
knows better?

Which of these is worse code?

This:
* process customer record
exsr prccus

prccus begsr
...500 lines of monolithic code that does 20 things and uses dozens of
global variables and indicators
prccus endsr


or this:

// process customer record
callp processCust

processCust PI
...500 lines of monolithic code that does 20 things and uses dozens of
global variables and indicators
processCust e

Better code comes from better programmers.
Better programmers have better style.
I don't see how it can work any other way. Forcing my people to write
in Spolsky's style won't make them as good as Spolsky.

I think code reviews have value in any group - when they are used to
disseminate and advance the state of the art.
--buck