Checklist for Code Review of Web Applications
Code review is a healthy exercise that should be performed on a regular basis, and helps in improving engineering sophistication of team members. Following are common best practices that should be applied to all code built for web applications built in any language or framework. Most of the points covered can be apply to other development domains, such as mobile applications as well.
- Use meaningful names for functions, and variables
- Avoid using globals
- Avoid using singleton pattern. Pass the initialized object as parameter (to method or constructor) instead wherever possible.
- Stick to a strict coding style and use code editor’s built-in helpers to format code every now and then.
- Avoid keeping commented out code. Remove it! Version control system already takes care of keeping the history for reference later.
- Comment as much as needed but not more (avoid stale comments)
- Break longer running function into smaller functions, and then break them into even smaller functions.
- Keep it DRY (Don’t repeat yourself). However, code should also have loose coupling, so don’t also over-do DRY! Code is DRY enough if you are defining helper functions within the appropriate model/view/controller layer.
- Avoid heavily nested code (using nested for loops, nested if/else conditions)
- Build on the shoulders of giants. Use existing developed open source libraries wherever possible to ensure flexibility/robustness
- Avoid making SQL calls directly from controller or views. Use models
- Parameters of model function should require minimal (if any) validation. For the most part, all the validation should have been done in the layer above (like controller or lib function).
- All SQL queries should use an appropriate index. Composite indexes should be introduced wherever possible.
- Separate controller layer from view layer. At the very least, controller functions and view functions should be clearly separated.
- Public function/interfaces/controller methods should always validate the parameters received. This is not mandatory for Private/Protected functions, so only expose those function/interfaces as public that provides such guarantees.
- Views should be free of business logic. Conditional statements should be avoided through creating sub-views and calling the appropriate view through the controller.
- Avoid using conditions in views (or view functions). Conditional views can be avoided by ensuring that controller calls the appropriate view (or view function)
- See error handling as a separate concern. Don’t mix it with the program’s actual flow
- Use exceptions rather than return codes. Abusing exceptions is however also not encouraged, and return codes are generally faster in practice.
- Avoid passing or returning null values.
- Verify the page loads correctly in all the major browsers (IE, Firefox, Chrome, Safari)
- Ensure that appropriate maxlength and size limits are applied to input fields of the form.
- Ensure that when a functionality is changed, you have identified all the ‘callers’ of the functionality before hand. Ensure that the change in functionality works for all the callers (be it be other standard functions, interaction points such as ajax call or other html GET/POST requests).