When is the most effective time to do code reviews?

Question

In the process of adding a new feature, when should you review the code? If you leave it until the code is complete, arguably a lot of time could already have been wasted going in the wrong direction. Too early and the ideas are only half formed. So where is the sweet spot and how do you decide?

Answer 1

Whenever we complete a feature, before we begin testing said feature, we will do a code review, except very briefly. This gets most of the ugly code out of the way, then on a weekly basis, we perform full code reviews of all features since a previous iteration (we are an agile shop), any features that don't have a good code review and testing are not considered complete in our bi-weekly iteration.

Answer 2

I'd say it's best to do it throughout the development cycle, particularly for newer developers whose practices you might not trust.

I had to submit a design document to my reviewer before writing anything but prototype code so any bad architecture decisions could be caught early on; an extra benefit was I got good experience following our group's practices. After that review, I had a few review sessions as I implemented the code.

I feel like there's no reason there shouldn't be several reviews from the design phase until a final review.

Answer 3

My take is that you should review at a high level early in development, focusing on design and architecture, and review at a lower level late in development, focusing on efficiencies and technique.

How many reviews is up to you, depending on time available and other factors.

Answer 4

Ideally, your features will be broken up into smaller pieces so that any one "task" is no more than a few hours of work, maybe a day. When any one of those pieces is done, and has been unit tested by the developer doing the work, that's the time for a code review. You shouldn't have people "going dark" for weeks at a time on a complex project with no check-ins or reviews during that time.

Answer 5

You have to have code to review -- so you can't do it too early. There's just no point in reviewing code that isn't done. Reviewing unfinished code means you are wasting too much time.

Review code when the coder thinks they are done with a discrete feature or bug-fix. Then hold them accountable to follow up and make changes recommended in the review.

The purpose of code reviews is not exclusively to fix the code being reviewed. This is a chance for team collaboration. After you spend some time doing it, hopefully these reviews will turn up less problems and more insights across the team.

In some cases when working to train a Jr. Dev, you can "review" their code through mentoring and pair programming, but that isn't an official code review.

Check out this question for some code review resources. Also check out code collaborator.

Answer 6

The best time to do a code review is NOW. As long as the other party has a break in their day, code reviews are least effective when done at the end like an after thought. They should be part of the development process.

Nothing is worse then reading pages and pages of another person's code. It is just human to start skimming and not putting 100% effort 100% of the time

Answer 7

I've found the most effective form of code review is real-time code review; that is, review it while it's being written, as an active participant in the design and implementation of the code.

Answer 8

I think the direction for any new feature should not become apparent after the code is written. That is to say, any new systems actual implementation should not be a surprise to interested parties. Previously I've used a proposal system, which I still do even if the team I am on don't adopt it.

Basically the feature has a high-level brief, an explanatory diagram (could be UML, flow, or whatever visually represents the proposed system best), and some implementation-level discussion on tricky algorithms. One key element to make this work is to try come up with two solutions, and describe the limitations of both while arguing for one of them.

This helps prevent people from becoming so involved in their first idea that they don't consider alternatives.

The proposal can then be commented on by all interested parties, and questions asked (wiki does well for this). The original authors might revise the proposal based on feedback once or twice. Implementation begins if there isnt a particularly big 'gotcha' pointed out.

Specifics of the implementation (eg: code) only need be reviewed when the first usable implementation appears and testing can be done on the code. One assumes that the individual(s) are required to follow the revised version of the proposal they came up with.

Answer 9

In my experience the best time to do the code review is right after the programmer has finished coding a reasonable size feature/iteration, and done the unit testing necessary to see that the feature actually works. These two things should have taken the programmer no more than 5 days of work, or else you are looking at too much code to review for the this step in the process be done thoroughly.

Your programmer should be able to realize when no progress is being done in a task and ask for help before the allocated time for the task is out, so in theory the "correctness" of the solution should not be the focus in the code review as much as the way the solution was reached, the coding style (in case your company has them in place), readability, and documentation. You might also want to look at what unit testing as part of the code review to ensure that a reasonable case was not omitted.

Answer 10

The most success I've had was scheduling regular weekly reviews for small groups - 4-5 people. Everyone would bring their code, and everyone would get picked on.

The worst experience I've had is when we used to wait until the end of a project to do them. At that point it was too late to make changes, and there was way too much code to effectively review.

Answer 11

G'day,

I'm assuming that you're reviewing the suggestions beforehand to see if they are wildly "off track" and a waste of everyone's time. ;-)

If so, then I would assume a review of the design would be in order.

Depending on the "size of the revelation of what's missing but essential" (-: I would be having a review of progress and updates in an agile way during development.

Nothing worse than having someone beaver away in the dark for several days/weeks/months only for you to find that they've implemented something in completely the wrong way/technique/style/etc. )-:

In addition, a code review, using Fagan type, egoless techniques is essential before the code is deployed to live.

HTH.

cheers,

Rob

Answer 12

A code review is done when the code is feature complete but while it's still fresh in the implementor's mind (otherwise she will have a hard time explaining certain decisions).

A design review is done when the design of the feature (how it's going to be implemented, what modules are involved, how it's called, who it calls, what global/local data it uses, impacts on processing, other modules, I/O, what requirements it meets, special corner cases, etc) is complete and documented.

The design review should give enough information that people can tell if this is the wrong direction before the code is implemented and debugged.