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'm quite new to programming design patterns and life cycles and I was wondering, what should come first, code review or testing, regarding that those are done by separate people?

From the one side, why bother reviewing code if nobody checked if it even works? From the other, some errors can be found early, if you do the review before testing.

Which approach is recommended and why?

share|improve this question
1  
note that the question is about the sequence of these steps, not whether they should be performed at all –  Richlv Feb 11 '11 at 16:22
6  
If you where using TDD your question wouldn't even make any sense. –  Crazy Eddie Feb 11 '11 at 17:29
add comment

9 Answers

up vote 32 down vote accepted

Developer unit testing first, then code review, then QA testing is how I do it. Sometimes the code review happens before the unit testing but usually only when the code reviewer is really swamped and that's the only time he or she can do it.

share|improve this answer
1  
That's a fine way to approach it. Just want to add that it's also valuable to code review the test itself (mainly to spot coverage gaps). –  Kevin Hsu Mar 22 '12 at 23:20
    
@KevinHsu, excellent point –  HLGEM Mar 23 '12 at 14:07
add comment

Our standard is to do the code review before the product goes to QA. The reason for that is that once the product has been tested and verified, there is less incentive to do refactoring and otherwise modify the code internals. It would then have to all be retested. Note that we also do unit testing in most cases.

share|improve this answer
add comment

Ideally, in an Agile world, both :)

Test-driven development is a method which encourages the development of unit tests prior to the writing of actual code - this way, you can capture the specification in code and write tests that pass the tests. Following that, automated integration tests that ensure all the various different components fit together are a Good Thing to further ensure the functionality of your application matches what is expected.

As for code reviews, pair programming is a useful way of having another mind overlooking your code as you're actually writing it. However, that's not necessarily a practical approach. The way it works in my current company is that code is reviewed after it has been tested on the developer's personal machine, but before it has been deployed to a shared development server.

share|improve this answer
add comment

Code review is done to "polish" things that already work, to assure the code has the desired quality level and meets the code guidelines defined by the company.

Code review can also happen as part of the future general optimization activity where you refactor and improve the old code.

If you practice code review before doing a check-in then code review falls between two testing stages: you as a developer test your code first, your peer does code review, you check it in, then later dedicated testers will perform more thorough individual and integrations tests.

share|improve this answer
add comment

Test first. Test last. Test, test, test.

Code review is nice-to-have. But difficult - can be a painful process if personalities involved or differing opinions.

Testing is very clear: either it works or it doesn't work. So test, test, test! And code-review if possible.

share|improve this answer
    
And when to sleep? –  user8685 Feb 11 '11 at 16:14
2  
@Developer Art: When your test cases are running. :-) –  Chris Feb 11 '11 at 16:54
3  
Code reviews can catch things that testing can't, and can involve significantly less time. At the very least, it's good to build a rapport with another dev and review each others' work. Plus you learn so much from their code when you return the favor! –  Ethel Evans Feb 11 '11 at 18:02
    
Disagree... Code reviews are vital, not only for finding subtle bugs but for discovering style errors, and performance bugs which a experienced programmer can detect by looking but will take testing a lot of time to find –  Amit Wadhwa Feb 12 '11 at 8:00
add comment

At my last job we had three different types of code reviews that we would do at different stages of the product lifecycle. The first type we called a Sanity Review, and it would happen before a developer had even done unit testing- in fact Sanity Reviews were done even before features were completed. The idea was that a pair of team members would sit down and just go over a few random sections of code as it was in the development process, to make sure that development was going well and we weren't going to end up with a giant TDWTF entry once the feature was ready to be merged in. This was done mostly for people who needed extra guidance (junior developers, new hires, and people assigned to work on something they were less familiar with than other team members), and was generally kept to a short meeting that only addressed egregious problems.

Next we had unit reviews. These were generally done with three developers and would be done when a unit/feature had been tested and was ready to be merged into the main tree. This was the meatiest review and would go into quite a bit of detail. We had three developers for this because we had the original author of the code, the tree maintainer who had to sign off on the code before it could be merged in, and a third developer who had been selected to be the backup to the original developer (the idea being once a section of code had been completed, there should be a full knowledge transfer to one other team member, so there were always at least 2 people on the team who were fully comfortable with any part of the code base).

Lastly, we had project reviews. This encompassed the whole team and would take about a week, and was done after QA and product launch, and the goal was to have everyone sit in and walk through all of the changes to the whole codebase over the last release cycle, where everyone could talk about architectural changes, gotchas, and decide what needed to be refactored and fixed before we began development on the next version of the project.

share|improve this answer
add comment

First, write automated tests for the code to be developed. Review the tests to make sure all known requirements are being tested. Write the code. Review as often as desired.

If any manual testing is required, it is critical to review the code before any manual testing is done. Otherwise, design improvements suggested in the code review will be rejected because the manual tests will have to be rerun.

share|improve this answer
    
And what about review? It also has to be redone after code has been changed, after the testing (if errors were found). –  Silver Light Feb 11 '11 at 20:51
add comment

Which is first, the egg or the chicken?
It depends.

If you are new and not sure of what you do, then by all means ask a peer to give you a bit of help. This is an informal but very serious and valuable code review.

Generally though I would suggest that you do your own dirty work first, make sure you have ironed out the code, commented it well in the right places (i.e. the tricky bits, not the obvious ones), it at least basically works (you have tested at the very minimum general cases and some limit cases or exceptions). Then you take it to your peer.

Getting your code reviewed too early could end up in a terrible waste of your peer's time. Getting it reviewed too late could end up in a terrible waste of your time. You need to find the right balance for highest efficiency. So some tests go first, then the review, then more testing. Potentially you may have several code reviews, depending on complexity and iterations, with different purposes and focuses.

The less sure you are the more reviews (when you are in your early learning phase, this is normal). The more sure you are the more reviews too (it is never good to be too sure of yourself, that means you generally are not quite as good a team player and could land others in trouble, you need to make sure your code can be understood and used by others). It is when you are in the middle that reviews can be spaced out some.

Only my two cents.

share|improve this answer
add comment

Capers-Jones, who has studied and measured the resultant efficiency and quality of software development processes more than anyone else, recommends the following sequence of defect removal activites:

  • Design inspections
  • Code inspections
  • Unit tests
  • New function tests
  • Regression tests
  • Performance tests
  • System tests
  • External beta tests

One of the reasons for conducting the code inspection ahead of the testing is so that the review can consider not only the code itself, but the testability of the code.

share|improve this answer
add comment

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.