Code Reviews – How to Choose What Code to Review

code-reviews

I am part of a team of seven developers in a small software company and I am trying to introduce regular group code and design reviews. We have carried out some reviews in the past, but it has been sporadic. I would like to make it a more regular thing.

I have read Code Complete and other similar resources and they talk about the mechanics of how to carry out code reviews but I have been unable to find any best practices on how to choose what to review. We have a code base that is more than eight years old and covering a variety of languages, so there is plenty that could be looked at.

Here are some of the factors that I can think of that might affect the choice:

  • Language: C, Java, SQL, PL/SQL
  • Code age: New code vs old code
  • Code usage: Frequently used code vs (effectively) dead/little used code
  • Code importance: Critical code vs non-critical code
  • Developer: Junior developer code vs Senior developer code

I understand that this isn't a question with an absolute definitive answer, but any guidance would be useful.

Some peripherally related questions:

Best Answer

In general, you must review everything. If a fresh application has 2 000 LOC, all 2 000 LOC must be reviewed.

That's why there is no best practice on how to choose what to review.

If you approach an existent large codebase, never reviewed before, then it's the same thing when you must rewrite an existent large codebase, and to choose where to start. It strongly depends:

  • on the codebase (a single monolithic code would be more difficult to rewrite/review than a set of separate components, etc.),

  • your context (can you stop everything you work on and spend three months (three years?) working only on rewrite/review, or you must do it by small lapses, only when you have free time)?

  • the type of review you do (do you have a checklist of things to review? Depending on the items of the checklist, you may want to review some parts first).

If I were you, I would:

  • follow the 80%-20% principle, mentioned in the first comment of the second question you linked to.

  • take in account that 100%, being an ideal, isn't maybe worth it. It's like 100% code coverage for unit tests, except that such code coverage is mostly impossible or extremely expensive.

  • start with the parts of the code you use the most and which are the most important. If the codebase has a library which authenticates and registers new users on your corporate website, review it first, because you want certainly find security holes before hackers do.

  • use existent metrics to determine what is more important to review. If a part of the codebase has no unit tests at all, while another, equally important part, has 85% code coverage, start by reviewing the first part. If a part of the codebase was written by a developer who was known to be inexperienced and to introduce more bugs than any of his colleagues, start by reviewing his code first.

Related Topic