Benefits of Code Reviews
- You write better code when you know it will be reviewed.
- A second (or third, or fourth) set of eyes will help spot defects. This is very similar to pair programming, but it works even better if you're working with an offshore team. It's also a great way of learning new APIs. For example, someone could tell me that my code can be easily done using the Guava library, or that the code is actually an "aggregator" Enterprise Implementation Pattern, and I should probably look at Camel.
- More than one person understand your code (cross-pollination or avoid silos). Having more than one person look at your code helps spread the knowledge and context of the problem or solution.
- Reducing the learning curve for new developers. I believe that even junior developers should be part of the code review process. It's a great way for them to learn about the code base and they become more productive.
What To Look For
- Bad design. Highlight issues such as SQL injection, look out for lack of design patterns, or anti-patterns. Things like separations of concern, encapsulation, and apply certain basic OOD principles - DRY, encapsulates what varies, open-close principle, etc.
- Performance hazard. For example, memory leaks.
- Lack of clarity - the application should work and the code should be readable. For example, a class named "SomethingServiceImpl" with no documentation on the class will be highlighted and will prompt a change request to the developer. Also, a big nested if statement that is not quiet clear will prompt a change request.
- Not consistent or not according to standards. Having a set of standards makes code reviews a lot simpler. It also sets a norms for the team. For example, not using Domain-Drive Development standards, consolidating APIs (Guava vs Jakarta Commons), having a handful of languages, and having a code style rules.
What Not To Look For
- Premature optimization. Don't try to optimize all of it at once. As my buddy Tyler mentions, "make it work, then make it better".
- Skills and expertise gaps. In our company, we allow all developer to do code reviews, including junior developers. These developers gain a lot of knowledge about doing code reviews.
- Personal style. If the CTO goes over and says, "I wouldn't do it like that" it bring noise to the process.
Quality and Dissemination of Knowledge97 Things Every Software Architect Should Know,
Chances are your biggest problem isn't technicalMost projects are built by people, and those people are the foundation for success and failure. So, it pays to think about what it takes to help make those people successful.