Code Review Checklist and Guidelines
Many entrepreneurs owning software products hire vendors to perform a code review of their applications. There are some reasons why they do that, but unfortunately they rarely specify their goals.
It is not clear if there are generally recognized best practices of code review. However, as an engineer who prepared many code audit reports I have my own vision of what clients may want to know about their stuff. So if you are one of those who has a code base or a live software, here are a few thoughts that may help you to get more out of your code review.
One of the aspects you may want to be confident in is security of your application. Security is absolutely vital for applications in software development areas such as Healthcare or ERP, but it is also important for other projects that store personal information.
The point that a small web project would not be hacked because of low presence in the Internet is not valid these days. Web applications are hacked not by people, but by robots that know how to approach most of platforms. When a public source shares information about a new vulnerability in one or another framework, all robots get their scripts updated. Usually, framework authors close the backdoor quickly, but one should make sure that his application uses the latest version of the platform.
This way a code review should detect common gaps in the code such as SQL/HTML-injections or mass assignment vulnerabilities (the reviewer may use some code review tools to generate a list of problems). Also, it should tell whether the application uses a secure version of the platform or not.
Stability of an application is another reasonable-to-ask question, because stability represents quality. Some customers ordering a project pretend that they want a “bug-free application”.
There is, however, a difference in what is called stability and what they mean by “bug-free” (by this buzzword they simply mean that they would want a free maintenance period after the application is released). Bug-free applications are not real. An application can be “bug-free” if it has 0 users, but if there are real people who perform some real actions, issues will happen. This is why famous products like Skype, Gmail and so on have support teams and full-time developers.
At the same time, stability is something achievable. Stability means that the project does not crash without any appropriate reason. It does not hang for a long time. It does not exhibit bugs which are almost impossible to reproduce.
As opposed to bugs caused by user actions, these nasty errors happen because of low quality of the code. In other words, the code has severe mistakes such as memory leaks, deadlocks, undefined behavior and so on.
All these unsafe coding constructions are apparent to a skilled reviewer, so you may expect them to be pointed out. Getting rid of this stuff is a good goal for you, because no one knows when it fires and shoots your application to death.
One of my colleagues once said that software developers can follow any approach and use any technical means in any project which lasts less than a year. After a year it becomes obvious which of their decisions were correct and which were not.
It leads to further speculations. If a project should last for two months, developers may use whatever they want and even violate common programming patterns without thinking about consequences. They would rather focus on speed instead. If a project should last for two months, it should not necessarily be “scalable” (another buzzword some customers mention in their job descriptions).
However, this attitude results in an unexpected outcome. Projects developed in this speedy manner are usually not maintainable. If a developer cares about future updates in the code, he should dedicate significant time to writing automated tests and specifications. Also, following development patterns becomes a question of principle, since patterns are good not only because they are an effective code, but also because they are readable.
On the other hand, if the developer did not have a goal to make the application maintainable, the application will not be such.
In Anadea we work hard to make our code readable and transparent to other engineers. This is why one of the aspects I analyze in my code reviews is maintainability of the application. I assume that if the clients request an audit, they are interested in an evolution of the code base. Hence, they may be interested in a time range allotted for a new developer to get onboard.
If you have the same purpose for requesting a code review, you may state your goal to the reviewers and ask them to focus on this aspect. By the way, if you want to turn your non-maintainable code into a maintainable one, there are some ways to do that if you know how one or another feature is supposed to work and are ready to share the information.
If you find this code review guidelines useful and would like to know more about security, stability and maintainability of your website, you may go to our free code review page and send us a short message. We will be happy to get a note from you.