How do you review large blocks of new code

Question

I'm a strong advocate of code reviews, whatever that means. I know it means different things to different individuals and groups and can be applied differently in different phases of development.

When I make a one-line change to a #define to fix a typo in a user-visible prompt string, "Hey, Joe, did I spell 'FooBar' right?" is easy. When you make a few related changes in several related functions, an over-the-shoulder sanity check is 10-15 minutes' work.

But what about a new project with tens of thousands of lines of brand spanking new code? It doesn't happen that often so I'm not as comfortable with it. How do you review that? One-on-one? Hand off and review "offline" with later feedback? In a group setting?

For the different approaches, are there any rules of thumb for lines/hour reviewed so I can estimate the time to allow for it?

Answer 1

I think it is recommended to do code review before it reaches the thousand of lines of brand new code. After that it becomes the problem you're facing.

Answer 2

Personally, when I'm responsible for another programmer; I review source code check in diffs as close to daily as possible

This is providing everything is in a working state. If their changes are not in a working state, I will wait until they are.

If they take more than 3 days to get a change to a working state ... I take it as a warning flag that needs to be investigated. This is usually something simple like making too many changes at the same time or a major refactoring with too many implications.

Answer 3

If the code is pretty complex, then I like to look through it with the original developer one-on-one. That way I can ask them to explain how it works instead of taking the time necessary to figure it all out by myself. I use a diff tool to see what has changed in multiple files and we walk through the flow together paying close attention to the changes.

If the code is pretty straight-forward, then I normally look through it by myself. I only call in the original developer if I have questions or concerns. Once again, I use a diff tool to see what has changed and walk through the flow.

If there is a ton of new code/changes, then I normally only review the critical path and spot check other parts of the code. In this situation, I am not trying to find every thing that might be wrong with the code. My assumption is that testing will catch most of the bugs. My review is mainly to catch any major bugs, to make sure that the code follows basic good software practices and that it will be easy to maintain.

Answer 4

Ideally you would do a code review when it is being built, so you can provide feedback and reduce issues significantly.

That said, I would use static analysis tools. That can check for regular issues and even do general analysis on the dependencies in the code.

And the very first thing would be: does it have unit tests? Start there, check they are well understood, it can reveal tight some coupling issues quick.

If the code is for an existing system you are working on as a team, you should consider:

• Continuous integration + commit small pieces of change (instead of doing a big commit after several days)
• TDD - focus the team on doing unit tests, specially to avoid tigh coupling
• Have not only unit tests, but other type of tests run periodically. Have different type of tests separated, you don't want load testing triggering on each build for example.
• Have static analysis tools run with the build
• Analyze the build info as a first step, before going further with the code review.

Answer 5

My approach is to decompose the work into smaller chunks and review the changes as soon as possible. It's much easier to review 100-200 lines of code than it is to review several thousand and you can propagate any changes you need to make to subsequent code.