Though the code review should not be a very complex process it does have its advantages of preventing build failures with preventable code mistakes that your peer could easily find. Over this advantage I always felt that the greater advantage was - the communication between the engineers. The person whose code was being reviewed started to see what and how the Code Reviewer looks at the code and its side effects. It also helped transfer the coding best practices. In case of a Code Review being done by a Senior Developer it also started to set up a reputation for the junior Developers.
Though the Code Review cannot prevent all the problems it does set a minimum level bar for the code going to the version control system. The Code Reviewer should be called in only after all Unit Tests pass and the developer has done testing and step-through of his code changes. Following are the points that our Code Reviewer takes care of:
- The code passes the Code Reviewers hallway testing.
- The code should not be repeated at multiple places. An experienced hand can point to other places where similiar code may exist or can be easily refactored to accommodate the changes.
- The code does not break the encapsulation of modules or introduce wrong dependencies.
- The code is maintainable and readable with proper variable/function/class names.
- The variables/functions and classes are at the minimal scope and visibility possible.
- The code should be minimal lines without being esoteric.
- The code should be commented at all places where an average developer needs to think hard as to what it does.
- There should be no hard coding of magic numbers
- There should be nothing in the code which prevents the same created build to operate on local machine, staging and production environment.
- The code does not introduce obvious inefficient constructs.
- The code written is always internationalized.
As you may observe all these are language/technology agnostic we have applied them with C++ and now in Java enterprise. What else do you think needs to be added without restricting engineer's creativity or making Code Review a complex process?