aspose file tools*
The moose likes Agile and Other Processes and the fly likes What is your view on code reviews? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Engineering » Agile and Other Processes
Bookmark "What is your view on code reviews?" Watch "What is your view on code reviews?" New topic
Author

What is your view on code reviews?

arulk pillai
Author
Ranch Hand

Joined: May 31, 2007
Posts: 3262
What is your view on code reviews? Why many companies don't recognize the importance of this? at least in my experience.


500+ Java Interview Questions and Answers | Java job hunting know how & Java resumes
Rajkamal Pillai
Ranch Hand

Joined: Mar 02, 2005
Posts: 443
    
    1

Hi,

IMHO code reviews are important in software development as more often than not, the one who develops the code and the one who maintains it wont be the same person. The reviewer ideally goes through the code and

* checks for the readability of the code (necessary comments),
* checks for the careless/clumsy coding style (unwanted/unused variables, coding style etc) and finally,
* checks for other necessities as laid out by the processes followed (in the particular firm).

The review process standardizes the coding style followed by different developers and tries to attain a similar coding pattern across the firm. Another reason I can think of is that while coding everyone (me, at least) tend to leave out unused code during the debugging and bug-fixing phases which in the end manage to land up in the production code (some, if not all) if the code had not been reviewed.

In all the firms I have worked with there was always a peer review before the code was released to the QA. I guess it depends on the firm to follow it or not.

Cheers,
Raj.
[ September 24, 2008: Message edited by: Raj Kamal ]
Alaa Nassef
Ranch Hand

Joined: Jan 28, 2008
Posts: 467
I agree with Raj. Even with metric tools and style checkers, the human factor is really important to ensure clean code.


Visit my blog: http://jnassef.blogspot.com/
arulk pillai
Author
Ranch Hand

Joined: May 31, 2007
Posts: 3262
Raj, in your experience, do you see this properly practiced or policed. I know that the product development or consulting companies police this better.
Katrina Owen
Sheriff

Joined: Nov 03, 2006
Posts: 1364
    
  17
Originally posted by Raj Kamal:
The reviewer ideally goes through the code and

* checks for the readability of the code (necessary comments)
(...)


Hi Raj.

There are many other factors to readability than comments.

Comments can occasionally be helpful, but in my experience, most comments are either pointless, misleading, incorrect, unhelpful, badly formulated, or overly verbose.

Well-written code should be readable in and of itself. Comments are great when you need to tell the reader about trade-offs you considered, or why you chose to implement something the way you did (briefly!), or gotchas to be aware of.
arulk pillai
Author
Ranch Hand

Joined: May 31, 2007
Posts: 3262
Originally posted by Katrina Owen:


Hi Raj.

There are many other factors to readability than comments.

Comments can occasionally be helpful, but in my experience, most comments are either pointless, misleading, incorrect, unhelpful, badly formulated, or overly verbose.

Well-written code should be readable in and of itself. Comments are great when you need to tell the reader about trade-offs you considered, or why you chose to implement something the way you did (briefly!), or gotchas to be aware of.



Well pointet out. Totally agree.
Adeel Ansari
Ranch Hand

Joined: Aug 15, 2004
Posts: 2874
Actually, practicing code reviews is as simple as setting up trac to generate a ticket for every commit and then assign it randomly or using some particular scheme.

Cheers.
Ilja Preuss
author
Sheriff

Joined: Jul 11, 2001
Posts: 14112
One word: Pair Programming


The soul is dyed the color of its thoughts. Think only on those things that are in line with your principles and can bear the light of day. The content of your character is your choice. Day by day, what you do is who you become. Your integrity is your destiny - it is the light that guides your way. - Heraclitus
Alaa Nassef
Ranch Hand

Joined: Jan 28, 2008
Posts: 467
That's two words
Robert Martin
Author
Ranch Hand

Joined: Jul 02, 2003
Posts: 76
Originally posted by arulk pillai:
What is your view on code reviews? Why many companies don't recognize the importance of this? at least in my experience.


Primarily because code reviews don't actually work all that well. Sometimes they can be made to work, but in the vast majority of cases they are a wasted of time and resources. The most common outcome of a code review is to leave the room hoping that the author knew what he was talking about because you sure didn't.

If you _truly_ want to conduct a code review, then every reviewer must be willing to spend as much time reviewing the code as the author spent writing it. No other level of review is worth doing. You have to get into the author's head and understand why he typed every single character in that source file.

Twenty+ years ago I ran a development team and instituted code reviews. Every author would surrender their module to a reviewer for comments. I gave the reviewers the code listing on paper, a yellow highlighter and a red pen. I told them to highlight every line of code as they read it, and to use the red pen to make edits and comments. The highlighter was to make sure that they had at least put their eyes on every line of code.

This was somewhat effective, but did not really find all that many bugs or issues. Moreover, the authors did not take many of the comments seriously.

Nowadays I think that pair-programming is a much better mechanism for code review. The two participants are both actively engaged in the authoring of the module. They share the keyboard frequently. They debate structures with each other, and act like two runners pacing each other. There is a certain competative relationship that forces them to do their very best.

Uncle Bob.


---<br />Uncle Bob.
Ilja Preuss
author
Sheriff

Joined: Jul 11, 2001
Posts: 14112
Originally posted by Alaa Nassef:
That's two words


Rajkamal Pillai
Ranch Hand

Joined: Mar 02, 2005
Posts: 443
    
    1

Hi,

Katrina: I agree 101% with what you say :-)
The comments part was just one of the ways to make code readable :-D

Arul: I feel the readability of the code should be left to the developer's discretion. Reviews probably could help making the code more readable but how do you police that? It becomes more crucial in product development firms because for them maintenance and customization goes on for the lifetime of the product with so many different developers being involved during the course of time. And for a different person to understand the code comments play a vital role, I feel.


Cheers,
Raj.
Adeel Ansari
Ranch Hand

Joined: Aug 15, 2004
Posts: 2874
Originally posted by Raj Kamal:
Arul: I feel the readability of the code should be left to the developer's discretion. Reviews probably could help making the code more readable but how do you police that? It becomes more crucial in product development firms because for them maintenance and customization goes on for the lifetime of the product with so many different developers being involved during the course of time. And for a different person to understand the code comments play a vital role, I feel.


Peer Reviews should work, I believe.
Roy Paterson
Smart Bear Support
Greenhorn

Joined: Jul 07, 2009
Posts: 6
(disclaimer: I work at Smart Bear, which makes a lightweight code review tool)

In my experience few companies do code review because historically code review was a very heavyweight and expensive process. Traditional code review involved long incredibly boring and tedious meetings, paper printouts, etc...

With the advent of good code review tools (like ours) much of that tedium and cost disappears and code review becomes much more practical.

Another factor is many people confuse static code analysis with code review. Static code analysis tools are great and everyone should use them, but they will never be able to match the value of having another human being actually look at your code.

- Roy


Smart Bear
Craig Suchanec
Greenhorn

Joined: Feb 25, 2008
Posts: 2
There is another aspect of code reviews that has been overlooked. I see code reviews as a place where developers can learn from each other. It should not just be a tear down session where everyone picks at every last detail. That sort of review just annoys people and does become counter productive and takes way too much time. That's not saying that a code review shouldn't point out flaws in thinking or bad practices etc, but not every line needs to be examined as if it is a clue to a murder investigation. I really have found that code reviews are more useful as a place to share ideas, experience, knowledge and learn from each other. Sometimes this means that the coder will be the person teaching, showing something they did in the code that is different, new, or unique. Other times it will be the other developers who help the coder by finding flaws in logic or by pointing out better ways to do things. If you approach a code review in a narrow focus of only looking at the code to pick it apart I think it becomes something nobody does, want's to do, or wants to listen to. However, if there is a focus on making everyone in the room a better developer then they are very beneficial. I have found this is especially true when you have many junior members on a team because those are the people that can learn the most. However, we all can learn from each other, no matter how good we think we are or how much experience we have.
Tauri Valor
Ranch Hand

Joined: Aug 03, 2005
Posts: 166
Most of the times code reviews proves to be futile unless the reviewer has deep insights into the topic rather than just pointing the correction.


A Moment's insight is sometimes worth a Life's experience.
Vyas Sanzgiri
Ranch Hand

Joined: Jun 16, 2007
Posts: 686

You are right code review should be with some one who has good understanding of the functionality. We conduct design and code reviews randomly - not to target the developer but to keep our repository clean and well documented. Shuffling code amongst developers is a very good way to automatically get the code review done - but in this case the Project Manager should be intelligent enough to know who last touched the code. Fortunately we have one that does :-)

We try to create very highly reusable software and libraries so such code and design review help - junior and senior developers learn from each other and documentation stays up to date. Unnecessary stress on unimportant topics and technology can be some disadvantages.


===Vyas Sanzgiri===
My Blog
Pat Farrell
Rancher

Joined: Aug 11, 2007
Posts: 4659
    
    5

Roy Paterson wrote:In my experience few companies do code review because historically code review was a very heavyweight and expensive process. Traditional code review involved long incredibly boring and tedious meetings, paper printouts, etc...


Yes, I've been in those all day long, 10+ people code reviews. Very painful, and very, very expensive. I was not convinced they were worth it, while good stuff came out of it, the cost was so high. Usually the developer would lose a week just preparing for the review.

I've heard that Google is religious about code reviews, but I expect that they are more frequent, faster, and just part of the culture.
Vyas Sanzgiri
Ranch Hand

Joined: Jun 16, 2007
Posts: 686

There is code review and then there is code critique. I think you are experiencing the latter..
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: What is your view on code reviews?