Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I am working in a team of 4 members. we have started reviewing code 2 months ago. we are using Gerrit as code review tool. while reviewing code initially we have found lot's of problem in the code i.e. no proper code comments. method are too big. no null pointer checking. no proper exception handling etc.

but now a days it looks code has improved. so when i am reviewing code i am not getting that much problem in the code.

so my question is should we continue code review even there is not that much flaws in the code? or will we stop?

share|improve this question
12  
same answer to when to stop unit testing. NEVER –  Bryan Chen Jun 25 at 5:53
    
If your team runs out of productive things to do, (i.e. running out of new features to implement, which is very common these days) then what choice do you have? Soon it will become checking the spelling and the naming standard of the variables. –  InstructedA Jun 26 at 5:14

3 Answers 3

You can do whatever works for you, but don't your code reviews show up real bugs (and not only "beauty issues")? I have seldom seen a situation where new code did not have enough bugs to justify a review.

Besides that, don't your reviews make sure there is at least one second person (apart from the author) who knows the code? And if you would stop doing regular code reviews, don't you expect the issues you have listed above will probably come back (at least, occasionally)?

So answer those question for yourself, then you know if stopping regular code reviews is a good or bad idea.

share|improve this answer
3  
Beauty is for Lint (or fxcop, or whatever code beautifier you have available to you). The review team should not be spending time with superficial concerns. –  Robert Harvey Jun 25 at 5:09
    
yes, that make senses. the issue i have listed above might come back and our code review also make sure there is at least one second person(apart from the author) knows the code. so i believe the code review should be a continuous process. great answer! –  Sharif Jun 25 at 5:12

no proper code comments. method are too big. no null pointer checking. no proper exception handling etc.

Some of these are just small problems that code review can catch.

Big methods and wrong exception are indeed problems, and it sounds like you are not doing unit testing (or at least not properly).

What is more important are big things :

  • design issues - it is important to detect design issues sooner, rather then later
  • to familiarize with other's code - you never know when someone is going to leave
  • learn something new - someone may suggest new ideas
share|improve this answer
    
yes, you are right. we are not doing unit testing properly. but we are doing UI Testing. using Robotium for android and SWTBot for eclipse RCP. i also agree with your important points that you mentioned. –  Sharif Jun 25 at 9:51

To put it clear, code review is not really for spotting bugs (even if it might do it), that is testing. Code review's purpose is mainly to keep project patterns, clean code, avoid local solutions.

To use a tool is useful, but it always worths the time and effort for a four eyes check to keep up good practices in the project. Even more senior programmers should let their code being subjected to a code review. The practices sometimes getting outdated or rethinked. In Java I can recommend Adam Bien's works on rethinking old patterns. If the team is heterogenous (various workplace past + age), the regular code review is vital to avoid chaotic coding.

Just to mention an obvious reason why to spend time on code review: if the code is reviewed and becomes homogenous in practices, anybody can go on vacation without constant bugging about their solutions. Since the code is similar, others can actually deal with even the unfamiliar modules of the program in case of bugfixing. Also if somebody leaves from the project, there will be no modules with horrible solutions which nobody understands.

So when to stop? Never. The regular code review pays back in long run. Makes the code easier to maintain.

share|improve this answer
2  
Part of the goal of code reviews is absolutely spotting bugs. Testing will never catch everything, so multiple defect detection techniques should be used. See Code Complete, Table 20-2 for details on defect detection rates of various methods. –  Mike Partridge Jun 25 at 14:52
1  
@MikePartridge I agree on a certain level, but I more tend to say, bug spotting is a by-product of code review. The main goal of the code review is a system of assuring the code quality. Auto and manual tests are for bug spotting. –  CsBalazsHungary Jun 25 at 15:05
3  
Code quality and defect detection are both products of code reviews. I recommend you read chapter 20 "The Software Quality Landscape" of Code Complete. It opened my eyes. –  Mike Partridge Jun 25 at 15:21
1  
Though I agree with most things you wrote, I would like to add one remark. The OP mentioned "no null pointer checking" and "no proper exception handling". Actually, I call this bugs, not just "messy code" or "local solutions". –  Doc Brown Jun 25 at 15:30
2  
I couldn't disagree more. What you are describing as the purpose of code reviews are the least important reasons. FINDING BUGS is the overwhelming reason for code reviews. The other stuff is just a by product. Code reviews take a lot of time. If the purpose of code reviews is what you describe then there aren't going to be many (if any) customers willing to pay for them. Customers will pay for code reviews because they believe it will find bugs sooner rather than later, like when they are 30,000 feet in the air. –  Dunk Jun 25 at 18:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.