tags:

views:

450

answers:

4

I would like to know how programmers do review of code written by others. I find myself bit sloppy in that respect. When ever a experienced programmer sends a request for reviewing code, I fail to be serious when going through the code just for the reason that I believe that he/she cannot be wrong. Most of the time I send back reply as 'No comments'. If the request is from not so experienced programmer I am better in reviewing but not entirely.

Recently, a bug was fixed and was released to the customer only to find another issue getting reported.It was later realized that if the team had done a proper review of the fix, the issue could have been avoided.

Any best practices for reviewing code and bug fixes done by others in your team?

-Prabhu

A: 

Use Static Code review tools such as CPPTest and Dynamic Code review tools such as Purify, memoryleakCatcher, etc.....

or Keep a Review Panel Team of 4 One needs to Concentrate on Testability One needs to Concentrate on Completeness(Tracebility) One needs to Concentrate on Presentation ( Coding convention, naming convention, etc) One needs to Concentrate on Logics.

....

lakshmanaraj
Do you have a link to this CPPTest ? All I could finf was about the eponymous unit testing framework.
philippe
is there something like this for perl cgi based scripts? thanks.
melaos
philippe -- www.parasoft.com - look out for cpptestmelaos, I am not sure on perl,, You may google or check with parasoft itself, they may suggest...
lakshmanaraj
+2  A: 

http://video.google.com/videoplay?docid=-8502904076440714866

google uses a web based review tool, there is also "Review Board", developed by vmware.

Nils
+5  A: 

You can try to work with your team/coworkers to build up a checklist of good questions to ask yourself while reviewing each other's code, ideally tailoring it to the languages you code in, the projects you work on, the domains in which your code runs, etc. In practice, I don't think it's reasonable to expect developers to pull out a spreadsheet and check it off every time they review code, but I think it's a good exercise to periodically do nonetheless. Additionally, if all code review "results"/responses are sent to the entire team, you can learn from the sorts of things that more experienced developers look for and spot during code reviews. (It also tends to lead to pretty good discussion.) At a few points, my team actually all got together to go through a sample code review, comparing each of the comments we had privately made...

As for specific questions to ask yourself, here is a non-exhaustive list of some of my usual ones (in no particular order):

  • Is the code correct, according to the design of the change?
  • Is the change approaching the problem in the right way--from a design perspective?
  • What is the context in which this code runs?
  • Are there any possible corner cases / boundary conditions that are being missed? Are all failure points / failure modes appropriately and correctly handled?
  • How hard will it be to test and maintain the code introduced by this change?
  • Will any behavioral changes introduced be surfaced to end-users? If so, will the end-users' experience be negatively impacted?
  • Is there a simpler fix possible that accomplishes the same work with negligible disadvantages?
  • How will this change impact the performance of the application/library/driver in question? Might any leaks be introduced?
  • Does any new code duplicate existing functionality present elsewhere in the software project, or perhaps in an external library that could be used instead?
  • How easy is it to understand the code? (Is it commented "enough"?)
  • etc.

The most important question for me is one of the first in my list--regardless of how good the code is, is the code change trying to do the right thing. Many times I've seen well-coded changes that don't leak memory, are performant, handle errors robustly, etc. but still aren't the right changes to make for the overall library/product/project.

Reuben
A: 

Ok, 1st thing 1st, remember anyone can make mistakes. Attempt to review/test the code with the intent of finding atleast one review comment or mistake.

Check the logic thoroughly, like 1st understand the problem the guy is trying to fix and then check for how you would have solved the same. Then compare your logic with his.

check the function, and c that it does what it is required to do and also check for the functions which will be calling this function works properly without errors

Raghu