Code Reviews
Code Reviews are an important part of development. I am a fan of having as many people as works involved because it is a great way for techniques and information to spread between developers. Remember that learning and improving together is the main goal of Code Reviews and it is not to show off how much more you know or to make someone else feel inferior. Listen to the coder and understand their reasoning. If there is an alternative way that may be better, introduce them to it and discuss it together.
Establishing criteria for evaluating code is important. It gives a framework to have the review and to guide discussion. When reviewing code I like to ask questions. Many times I am checking to see if the change was fully thought through. Why did you choose this implementation? Does your change match the style of the rest of the system? Is your change readable? Is the change maintainable?
These questions lead us to the four pillars I use for code reviews.
- Quality
- Readability
- Maintainablity
- Style
Over the years many principles have come from the software development profession. We can use this principles when developing and they will help guide us to construct software that contains all four pillars. This is not an exhaustive list of principles by any stretch but these are the ones I use in my code reviews. I am sure over time more will be added (and maybe some be removed).
- KISS
- Keep It Simple Stupid
- DRY
- Don’t Repeat Yourself
- SOLID
- Single-Responsiblity Principle (SRP)
- Open-Closed Principle (OCP)
- Liskov Substitution Principle (LSP)
- Interface Segregation Principle (ISP)
- Dependency Inversion Principle (DIP)
- YAGNI
- You Ain’t Gonna Need It
- Do not use magic strings
- Design Patterns
- 10 / 100 Rule
- Business Logic belongs in the Core Layer
- Display Logic belongs in the Presentation Layer
- Follow Coding Conventions
These are principles and not rules, sometimes the principles will conflict with one another.
An example of conflict: KISS and SOLID
- KISS could argue to make a simple procedural routine that reads input, processes, and writes output in a concise manner.
- SOLID could argue to seperate input, processing, and output.
Which is correct? Neither or both, depends on the requirements and constraints of the project. If this was one-off never to be used again code, KISS makes a lot of sense. If this was the beginning of a project that will go thru many iterations and has much unknown requirements in the future, SOLID makes a lot of sense.
If you are not code reviewing today, start. If you are the only person working on a project have a friend code review your work. Code Reviews are about people learning and growing together. I think you will find the time a good investment.