(This article was primarily written for another blog. The language of this article is more formal. Please let me know your thoughts through the comments.)
Writing better and high-quality code is essential. Better code makes software stable and seamless. Better written codes are also scalable and maintainable. And to write better code, code review is one of the most critical tasks. Code review is an excellent opportunity of improving code quality. In code review, developers ask for feedback before merging or deploying code to production. Getting a look by another pair of eyes into your code helps eliminate the blind spots left by one, and it also helps to find vulnerabilities before the code moves into production.
But when reviewing someone else's code, you always don't have to be an expert. Having experience with the programming language and a checklist can help you get started. Here is a list of things to keep in mind during code review.
Readability
Readable code is one of the essential features of good code. If a code is easy to read, it is easy to understand, making it easy to debug and maintain. Readable code means understanding what the piece of code does in the big picture by seeing the input and output of the code.
Unreadable code can make the work of finding a bug or maintaining a codebase lot more complicated. It is a huge technical debt. It can consume a lot of time for a developer or a team. In the long run, to make the codebase more readable, aka understandable, it is possible that they either break the system unintentionally while changing the code or delete the whole code and start from scratch.
Breaking down the codebase into smaller chunks can be a great way to start making a codebase understandable—separate codes by concerns. Long classes, functions, or methods can be tough to understand and hard to maintain. Break long classes, methods, or functions into smaller chunks of codes.
Another way to make code more readable is to naming correctly. Naming the methods or functions, classes or variables is a crucial thing to do. Names should be self-explanatory. It makes the code more understandable. If you are using abbreviations for any of your variables, ensure proper feedback is left for that.
Functionality
Functionality is another major important thing. The code has to do what it is intended to do. If the code is not doing what was intended, there is no reason to keep the unnecessary code. When we write code, it is essential to keep in mind that the end-user is both the developer and the user. Because, in the future, the developer is also going to use the code.
When reading through the code, it is essential to care about the edge cases and check if any bug is visible to the naked eyes.
It is tough to understand just by reading a code when a code has user-facing changes, like UI changes. In such cases, it is good to ask for a demonstration of the code change.
It is also essential to think about functionality when some parallel programming is going on. Because, in such cases, that could cause deadlocks or race conditions. These kinds of issues are hard to find just by reading the code. It is important to think through such cases carefully and make sure that these cases are not introduced.
Security
A security-centered mindset when writing code is very important. Prioritizing application security is very important. When reviewing the application code, check if the code covers the OWASP top ten. Vulnerabilities can also be introduced through third-party libraries or modules. You have to be aware of that too. Don't use out-of-date tools or tools that have known security issues. A small security bug can cost a lot of harm to a company's reputation.
Security itself is a huge topic. And putting yourself in someone else's shoes can help here. You have to think about what paths you would've chosen to steal data from the application. Thinking in such a way can help you find vulnerabilities in the code when reviewing. Make sure you also check for some proper security handling like
- Proper exception handling
- Security Logging
- If data encryption is handled correctly or not
Maintainability
The IEEE Standard Glossary of Software Engineering Terminology defines maintainability as,
The ease with which a software system or component can be modified to correct faults, improve performance or other attributes, or adapt to a changed environment.
Maintainable software makes it easy to remove bugs or add features without introducing new bugs to it. Maintainable code improves usability. In the long run, if the codebase needs to be learned by a new developer, an unmaintained codebase can cost a huge amount of time and can add a lot of technical debt.
Maintainable code means it also implements architectural principles like DRY, SOLID, etc. And it also uses design patterns and known algorithms to solve appropriate problems.
A maintainable code has to be well-documented. Check if the code README or the changelog is updated properly. If proper comments are utilized. When using comments, it is important to remember that comments must express why a piece of code exists rather than what the code does. The comments should always be made as simple as possible. However, in cases of exceptions like regular expressions or complex algorithms, we have to break this rule and explain the code. It is also essential to look for unnecessary comments that need to be removed.
Consistency is another thing to notice during the check of maintainability. Consistent code is maintainable. Check with the style guide of the company and be consistent with it. Consider the style guide as the absolute authority. And the code change should be followed by that. Though, there may be some cases where the style guide makes recommendations rather than declaring requirements. In such cases, it is good to be biased towards the style guide. And, if no rule is applied, then the code change should be made consistent with the existing code.
Performance
Checking for all the performance optimization is another important thing to have in your code review checklist. Performance has become one of the top-most priorities in building modern apps.
The first thing to check is, whether the code change is negatively impacting the performance of the existing code. Long database queries, unoptimized resources, or multiple unnecessary API calls can make the performance slower. Enabling lazy loading whenever possible is another performance optimization check. Also, caching should be properly used and optimized. You should also clearly check that none of the resources that are not used, is loaded, checking other than that make sure the code change relies on third-party-libraries as low as possible. Also, when working with the DOM, make sure the DOM reflow is minimized. Check the code for memory leaks. Memory leaks can be a huge contributor to performance issues. Loops are also a great contributor to performance. Making simple changes like calculating the array length outside a loop can help to boost performance. Look for such improvement areas.
Also, using tools like profilers can help to check for performance optimization checks.
Anti-patterns
An anti-pattern is a common response to a recurring problem that is usually ineffective and risks being highly counterproductive. The term, coined in 1995 by computer programmer Andrew Koenig, was inspired by the book Design Patterns, which highlights a number of design patterns in software development that its authors considered to be highly reliable and effective. (Wikipedia)
When working with junior developers, anti-patterns is a common pitfall that they fall for. Anti-patterns, as opposed to design patterns, is considered as bad practice in programming. For example, polluting the global namespace is an anti-pattern. When reviewing code change, you make sure that the code has no global variables. Polluting the global namespace can result in name collision. When the codebase becomes large, this can create a huge issue.
Extending the Object prototype in JavaScript is considered another bad anti-pattern. Extending the Object.prototype may seem helpful at times but when multiple teams start to manage the codebase, it becomes complicated and can create unknown side effects.
Knowing the possible anti-patterns in JavaScript can be really beneficial during code review.
Reliability
Reliability describes how fault-tolerant the code is. When things go south, reliable code shields the user experience from being ruined as much as possible. When writing code, we have to keep in mind that things will fail, resources will not load properly, API requests may return 500 requests occasionally. Writing codes keeping faults in mind can help build fault-tolerant code.
Reusability
Code reuse is a development process geared towards reusing existing code. This reused-based development process has gained popularity because it can lower the maintenance cost and increase the code quality.
Code reusability has multiple benefits. Like, fast development speed. Reusable components can be easily used across the codebase that can increase the development speed.
Dependability is another benefit of having a reusable code. A reusable code component must be well tested and its bugs and faults have been fixed over time.
Though, having its own benefits, it's always not a good idea to make every component reusable. Abstraction only when necessary is very important otherwise making a component that is not reused can increase the maintainability burden. It also takes more time to build a reusable component. Keeping all this in mind, check if the code change has only necessary reusable code components, or if there is any scope to make a reusable component.
Tests
Testing plays a significant role in making a product successful. Because of this, writing good tests is extremely important. A flawed test can be more dangerous than carrying no tests. Passing tests makes a developer feel more confident to push the new code to the production repo. But it is important to check if the tests are written properly. There may be cases when the code is passing the test for the wrong reasons.
And when writing tests, the rules that apply for code review also apply. The tests should be readable, well-documented, maintainable, performant, and should follow patterns.
Also, don't just think about the test cases that are written. There may be edge cases for which test cases have been missed. Think about that too when reviewing.
Conclusion
Reviewing code properly is an important task because the code quality depends a lot on it. Following this checklist can help you better review the code. And also, appreciate the good things that you see in the code change. Tell the developer when they address your comments in a great way. Code reviewing should not just focus on mistakes, but also focus on appreciation and encouragement.