Re: Do you do formal code reviews?

On 16-Nov-2011 10:13 , Voris, John wrote:

My own opinion is that a code review is nice, but designing or
retrofitting programs for *testability and* separation of UI
from logic has a better payoff in quality.

I second that. But I think a code review could concentrate on those ideas as well, with the same goal of quality improvement.

In my experience formal code reviews seemed to diminish in value, as perceived by the developers, mostly because of the tendency of the efforts to concentrate solely on the "functional" aspects of the code. In many scenarios, all who were involved in such reviews would tend to suggest there was little value and expressing little interest in such reviews, seemingly because everyone already "knows" that the code being reviewed already "works", irrespective the [il]legitimate rationale for that conclusion. Motivation for finding defects in something that is perceived to be functional is much more difficult than when either, the reviewers know the code does not work or nobody has figured out how to locate an origin for the apparent defect previously only identified by some failure and the associated error report\docs.

When reviews concentrate, instead of on functional aspects, on how the code could better enable [automated] testing or serviceability aspects, I think the outcomes from the reviews are generally better; better for all of: exposing existing defects [what a functional review intends foremost], identifying improvements to the implementation both functionally and testability\serviceability, and consideration for potential design enhancements. And for sure these same benefits also when reviewing code from the perspective of an intent to effect the decoupling of logic from the UI or from the mainline\monolithic code. That is, making actual changes is not necessary even if doing so might be very beneficial. Just by review of the code for the what and where such aspects could help, that alone could help to make a formal review worthwhile. Obviously without any actual changes, no improvements would result in software quality, however the reviewers are likely to both participate and gain more from the shift in focus for the process.

Although finding functional defects while reviewing for other than specifically the functional aspects of the code might not seem intuitive, actual attempts of such reviews as an alternative to the more typical functional reviews might prove fruitful. I believe I became a better reviewer by changing to look at the code in this way; finding misuse of parameters and variables, redundant coding, missed opportunity for caching, overlooked concurrency issues, and others, often things that I might never have found when concerned solely about the functional aspect. Although I admit I also became much more frustrated about the code, knowing that few if any changes aside from the strictest defect resolution would ever get implemented.

Regards, Chuck