Source Code Review Checklist: Signs of Quality Code

Many entrepreneurs owning software products hire contractors to perform a code review of their applications. There are reasons why they do that, however, they rarely specify their goals. Is it just because a code audit simply needs to be done? Or there’s something more to it?

It is not clear if there are generally recognized best practices of code review. This post you’re reading was written by Anadea’s software developers who prepared many code audit reports and have their vision of what clients may want to know about their stuff. So if you too have a code base or a live software, here are a few thoughts that may help you better understand what the code review service is all about.

Table of contents

Why do code review?

Resorting to intimidation is not the best thing to do, but in the case of code review, it seems like the best strategy. Just compare these two statements:

🤩 If you have your product code audited, software will bring joy to users as long as you want it to. It will be easily maintained and work fast on any device. Users will appreciate the top-drawer app and happy smiles won’t leave their faces.

Sounds nice but not convincing at all. Any entrepreneur thinks their app is already great without any audits. Now, let’s take a look at this:

🧐 Users abandon the app, leave negative reviews and choose competitors for which there seem to be no reasons. Code audit is the very first step to be taken to understand what’s wrong. Odds are high that mistakes in the code cause issues affecting retention.

That’s more like the truth. The thing is, most software is still written by people, meaning that human errors still take place. There’s nothing to do with this but to accept and try to identify problematic spots in the code. In the end, they say ‘four eyes see more than two’ for a reason.

jellyfish

If you’ve been at the Code Audit Service: Why You Need It page (highly recommend visiting), pass to the list of signs of a good code and find the code review checklist right after.

So, the good code is…

Secure

One of the aspects you may want to be confident in is the security of your application. Security is absolutely vital for applications in such software development areas 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 on the Internet is not valid these days. Web applications are hacked not by people, but by robots that know how to approach most 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 their 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.

Stable

Stability of an application is another reasonable-to-ask question, because stability represents quality. Some clients imply 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 achievable. Stability means that the project does not crash without any appropriate reason. It’s not freezing. 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.

Maintainable

There's a view 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 speculation. 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 clients mention in their requirements).

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, they should dedicate significant time to writing automated tests and specifications.

On the other hand, if the developer did not have a goal to make the application maintainable, the application will not be this way.

blue jellyfishes

At Anadea, we work hard to make our code readable and transparent to other engineers. This is why one of the aspects we analyze in our code reviews is maintainability of the application. We assume that if the clients request an audit, they are interested in the 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.

Readable

Readable code is understandable code. By looking at it, any developer not related to writing that given piece has to understand the meaning of each line and how it fits into the overall project canvas. Put it otherwise, programmers should read the code, not get through it.

Developers look at the code both from far away and up close. By far away, we mean the code should fit in the standard 14’’ laptop screen with no need to scroll it horizontally. Lengthy lines should be rewritten for better readability. And estimating the code from up close touches upon naming variables, methods, functions, and classes. Descriptive, non-random names make code more understandable not only to third parties, but even to the programmer who’s written it. We bet a lazy programmer who’s used to name classes a-la ‘qwerty’ forgets what it’s supposed to stand for in a week. Naming conventions (Pascal, Snake Case, etc.) were created not to be ignored.

Same about the uniformity of code style. Even though it doesn’t affect the code performance, consistency makes the code easier to read, as well as helps developers avoid non-meaningful edits. Removing a space here and there sounds as easy as winking but can eventually steal much time once the developer gets the taste for making these tiny improvements. 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. And normally, every team of engineers would have their guidelines to writing code (as well as code review guidelines).

Well performing

There are two sides to performance and speed, namely, from the users’ and computing resources points of view. What users understand by code performance is mainly page loading speed, which they can assess with not much effort. Lengthy queries, poorly optimized assets and multiple API requests slow down code performance and again show that ‘code is the staff of life’.

Code that’s well performing uses as much caching as possible and only loads what’s necessary to complete the query.

From the hardware point of view, there’s no need to store software in servers whose power is more than needed out of optimization concerns. Be practical and not put the system under more load than it needs.

In line with documentation

Project readme, if any, has to be in line with the current state of software—upon every update and significant fix, developers should make necessary edits to readme, or documentation, so as there’s sync between software and guidelines applied.

cloudy jellyfishes

Reusable

Reusable code can save developers time when new features are developed. It means that if you have an educational platform, a nice thing would be to make provisions that the code will allow adding more types of test tasks above the existing set of options.

In other words, there’ll be no need to alter the existing code when adding new features.

However, bear in mind that the desire to cushion the blow, or ‘future-proof’ the code is not always justified. There’s no way one can anticipate everything. The best thing to do is try to balance between making provisions for a reusable code and the ‘You Aren’t Gonna Need It’ principle.

Speaking of principles, we can't help but mention DRY—Don’t Repeat Yourself which you’re probably tired of hearing that suggests no duplicate pieces should be present in the solution code. Why is duplicate code bad? It greatly complicates the solution maintainability, i.e. when updating code, edits have to be made to every repeated element. It’s a waste of time and adds tech debt which you don’t need.

Coverable by tests

A test is a set of actions aimed to validate the code performance. We at Anadea write auto tests because they allow us to see that the functionality covered by them performs as expected. For each functionality, a separate test is written, so to make the code testable, it may need to be first refactored into separate, clearly divided functions, and the code audit service identifies such a need.

Same requirements as to code apply to tests, i.e. tests should be written in a uniform style, be readable, maintainable, and not repeated anywhere else in the code. Otherwise, having poor tests might be even worse than having none. Engineers should make sure every test fits for its purpose and is passing for the right reason.

Quick takeaways

Summing up, the points above boil down to these questions the developer auditing the code should know answers to:

  • Can I easily read and understand the code?
  • Are names descriptive enough?
  • Is the code well-structured and moduled?
  • Did programmers who wrote it follow common coding guidelines?
  • Does the code allow for desired updates without significant changes?
  • Are there elements used in the code more than once?
  • Is the solution functionality covered by necessary tests?

We really hope you won’t need to redo your entire solution after a code audit. But you never know! Send us a request to audit your software code and find out.