More On Code Reviews

Your friend and mine, Mario Aquino, has an excellent post on material for code reviews. In classic blogger tradition, why leave a comment when I can write a post?

I agree with his points, and will add my own below. Don't leave a comment saying 'what about unit tests?'. Mario has already covered a lot, so consider his post.

I once wrote about the Universal Issues that affect a software project. They influence this list.

Architectural Harmony

Mario writes about harmony within a source-file. One can expand that to the level of architecture: is this code in the proper place, with respect to the architecture? Is it in the right package/module?

Client-Side Specifics

Does the code follow known conventions for a GUI? e.g. In a Swing app, does it use the event-dispatch thread appropriately? What about i18n?

Server-Side Boundaries

Similar to the architectural harmony above, is the code correct with respect to transactions? concurrency? In a framework situation, these are often handled at an outer scope, but it is important to place ourselves in the appropriate context.

Concurrency is a fun topic because developers have an involuntary reflex: we look upwards to the ceiling whenever asked about multi-threading. Try it, and see for yourself.

Data Is Immortal

As I've said before, versioning is the toughest problem in software engineering, especially given that data lives forever. I've been on projects where data must persist across versions, and in this case, it is vital to give consideration to this aspect during a code review. The gotchas are downright ghostly and ghastly: i.e. hard to see, and expensive.

A Thought On Delivery

It is dangerous to riff through such a list in every code review. It will frustrate the developer and eventually turn people off from asking for code reviews. Though always wise to keep these things in mind, it is important to know when to ask out loud. That is a true art, and can only be gained by experience and a sensitivity to the working conditions.