Code Review Checklist

Here's a checklist for code reviews (in any language).

1) Would the logic be understandable by a developer of average ability proficient with other programming languages but not this one?

If it uses language features or obscure design patterns that are "clever" without good reason (e.g. something needs to have the maximum performance), then it won't be maintainable and someone will replace it or add to it badly. We've all seen code written by someone who has just read the "Design Patterns" book and wants to use all of them. If you want to use the latest trendy language features, think about if someone in 10 years time will know about these of if they will have fallen into obsolescence.

When reviewing code, thinking about this will make long term maintainability easier. It may result in some discussion about which language features and design patterns should be used and standardising on these will mean everyone understands each other's code better.

2) Are properties nouns and methods verb noun phrases.

If properties are not nouns and methods not verb-noun phrases, then it's a sign that these haven't been thought about carefully and the code will be more difficult to maintain.

When reviewing code it's easy to overlook this, but very quick to check.

3) Is there an obvious way of testing each method.

Of course we all know all methods should have unit tests but in the real world this isn't always possible. However, if there isn't one and it isn't obvious how to write one, then the method could probably be expressed better in a different way.

When reviewing code, if you can ask "how will this method be tested" you can better follow the logic. If you can't answer this question then there are more likely to be complexities that aren't picked up until it's too late.

4) Do the names of properties, methods and classes match the names used elsewhere in the organisation?

If the names of objects are different (e.g. your code refers to a regular customer and the business a star customer), then it will be harder to explain what you are doing and more likely that requirements will be misinterpreted. It's a good idea to document these in a wiki to have a single point of reference for these.

This also means if you are working on a method, you can say "I'm working on x in class y", and someone who knows your business area but who is not a developer know would know what you are talking about. This would improve the trust between developers and domain experts.

When reviewing code, thinking about this will help you understand the business domain and make you better able to communicate with them and get accurate requirements in future.

5) Is the total complexity of the system affected as little as possible? (I would define this as the number of classes * average size of each class * the total number of other classes a class has to know about).

This means if you're creating a new function that means the class it is in has to know about 3 more classes than before, the system is more complex as there are more dependencies between classes.

When reviewing code, if you think about this and rewrite if necessary it will reduce the numbers of possible points of failure and mean that testing is more likely to pick up problems before anything goes live.