TOP 3 Effective Code Reviewing Checklist

A few years back I composed an extensive checklist for things to consider while reviewing code. I started compiling this list after I saw that many code reviews were not as effective as it could be.
Each of the reviewers focused on things they believed were important (which is, of course, individual) or mainly commented on cosmetics (the easiest things to spot).
Lately, I was asked by the unit director to give my suggestion for "TOP 5 Effective code reviewing checklist". The first thing that came to my mind was "how is that even possible?" Reflecting on my past checklist, I was certain that are so many things worth considering. However, "if there are more than five items in the list", they said, "no one will follow it".  And they had a point. No one was using my checklist. Not even me…

After contemplating, I realized that I can summarize almost everything required for an effective code reviewing in this following TOP 3 questions

1.      IS THE CODE WELL TESTED? 

I'm not a fan of Test Driven Development (TDD) methodology, at least not in its extreme (Writing only the code that passes a single test). Instead, I suggest something else: Why not start the review by reviewing the tests first? You can call it: Test Driven Code Review.

This question forces the reviewer not only to understand what the reviewed feature is supposed to do, but to perform the review in light of the requirements. This is important especially in teams in which not every programmer knows every feature in the software.  Sadly, It is easy to review lines of code without understanding the bigger picture, and relying on the programmer that he (or she) adresses the requirements. Moreover, as it is easy to write procedures that test the written code (and not the requirements), it is easy to review tests in light of the existing code. This is handled when reviewing the tests before reviewing the code (and can help in preventing fixation and bias on part of the reviewer).

Secondly, I cannot stress enough that tests are the most important part of software development, in my opinion (I will save that for another post…). As such, it only prompts to be checked first, with the benefit of the reviewer being fresh and sharp in thought, rather than reviewing it last when the reviewer might feel tired and that most of the review is behind him/her. I truly believe that having the programmer know that the tests are the main focus of the review will also make a strong effect on the programmer.

Lastly, a common say is that "it is hard to maintain the tests". It can be true, but we should not cave to this… We need to demand the same high quality from our tests as we demand from the source code, mainly - easy to extend or modify. Therefore, it is important that the test code will be treated the same as the source code when we come to review it. Thus, readability and usability are important for the test code as well.
This question (that is, is the code well tested) along with automated build and code coverage, guarantee that

a.      The feature does what it's supposed to do.

b.     The feature doesn't do what it's not supposed to do.

c.      The code does not break existing features.

* Note that there are different levels of tests that need to be considered: unit, component, integration, performance and load tests and each level should receive the proper attention

2.      IS THE PRODUCT EASY TO MAINTAIN? 

This question addresses issues of support and maintenance after the feature is deployed, along with the entire product. I think this point doesn't get the proper attention as it deserves. I suggest the reviewers ask:

a.      Is the flow understandable from the logs? I recommend reviewing actual log files and not only loggings in the code

b.     How well are errors handled?

c.      What alerts are possible?

d.     Is it easy to monitor and control the feature?

3.      IS THE CODE EASY TO MAINTAIN?

Since we already know that changes are imminent, we need to verify that new code is easy to extend, change or fix, even by a different programmer than the one who coded it. This requires us to check its readability and design via means of:

a.      Coding conventions.

b.     Design matrices, such as: coupling and cohesion.

c.      Proper documentation.

d.    Readability, meaningful comments (that explains why rather than what).

These are my TOP 3 recommendations for an effective code review. But, since the world is not perfect, and since our tests will never cover everything, I suggest two additional (secret) bullets:

4.      DOES THE FEATURE WORK?
In my original checklist, this was my first and most important bullet. The reason why it is only fourth now (and secret) is that, intuitively, this bullet is supposed to be answered by the first bullet- if the feature is well tested and the tests cover the functional requirements, then the feature works. But, in practice, the tests will be incomplete and another level of quality assurance is required. Reviewing the code itself, seeking for bugs and making sure that the code covers all functional requirements, provides that additional assurance. Moreover, some of the bugs are really easy to spot in the code, yet are very hard to test (for instance, concurrency problems).

5.      DOES THE FEATURE HAVE SIDE EFFECTS?
While conducting a code review it is tempting to review only the lines of code that changed, rather than all the code parts that relate to the changed code. This is mostly because we use comparison tools to perform the code review which results in ignoring the related code that did not change.
If the entire software is tested properly, then this question is supposed to be covered too. In addition, well designed code will reduce unwanted side effects of code altering. Otherwise, we should check how the feature interacts with existing features, for instance:

a.      Do other features that use the altered code still work?

b.     Do assumptions change?

c.      Does the product's performance change?

To summarize my checklist for an effective code reviewing process:

1.      IS THE CODE WELL TESTED?

2.      IS THE PRODUCT EASY TO MAINTAIN?

3.      IS THE SOFTWARE EASY TO MAINTIN?

4.      (DOES THE FEATURE WORK?)

5.      (DOES THE FEATURE HAVE SIDE EFFECTS?)

Have a happy and productive code review!