Similar Topics...
 |
Things to Look for in a Code Review
Code reviews are important, but the time should be used wisely. The following should be your top priorities to check for, from most important to least.
#1 Is it secure?
If the software has security vulnerabilities, that is a worst-case scenario. It can result in great risk and harm to people and institutions. Bad actors can cause serious damage financially, socially, or criminally. You do not want to be the person who let this past your gate.
Skim through the code and spot check entry points and SQL/script executions. Particularly look for common mistakes, like:
- SQL/script injection
- Wrong/too much access granted to users or services
- Not checking inputs
- Missing server side validation
- Unsecured data, especially things like passwords, SSNs, or credit card numbers
#2 Does it work?
If the software doesn't do what it is supposed to do, not only is the customer unhappy, you and the team will lose time down the road when you have to reimplement and retest the feature. Check for the following in particular:
- Boundary conditions/edge cases
- Null checks
- Type casting
- Memory leaks
- Overflow issues (now or future)
#3 Is it performant
Developers are great at writing software that works in DEV for a small data set and later times out in PROD, leading to frustrated users. In particular, watch for:
- Network/SQL access done in a loop instead of in batches
- Datasets passed in entirety instead of in batches
- UNION instead of UNION ALL or DISTINCT instead of proper joins
- Multiple calls for the same data
- Moving or storing much more data than is necessary (e.g. Select * from table just to print a few rows or columns)
#4 Is it maintainable
Ensure code isn't being written that will take huge amounts of resources to maintain. Consider the following:
- Does this code generally use software stacks and design standards familiar to our organization?
- Is code being duplicated, which could instead be shared and reused?
- Is the code properly checked in to a source control system and release bundle?
- Is documentation for this feature required, and if so, is it up to date and easily found?
#5 Is it formatted well
Some developers incorrectly look at this first as it is the easiest to analyze and take issue with.
In fact, not only is it the least important, this is something I generally recommend you do not check at all . Why not?
- The user doesn't care
- Your company's money for your time could be used on other things
- Functionality is not impacted
- It is easy to upset others by questioning their programming style
At best, if someone's code is bothering you, casually mention it or put an occasional non-actionable comment with the suggestion. If it really bothers you, your best bet is to simply clean it up yourself.
Ultimately, not only are these important to the code review, they should always be in the mind of the developer. The best code review is one where no issues are found! If a developer follows these guidelines up front, everyone saves time by minimizing the back and forth of identifying and resolving issues. Even so, the code review is an important step in the release process, because no one is perfect. Having another set of eyes check your work not only results in better software, it increases team communication and knowledge by ensuring developers are staying abreast of each other's coding paradigms and techniques.
Created By: amos 7/28/2023 11:21:25 PM
Updated: 7/28/2023 11:56:13 PM
|
|
|
|
|
|