June 14, 2021 • 8 min read
In my previous article, we talked about what code review is and why it's crucial to the software development process. This time, we'll talk about what you should do when writing code to minimize your mistakes and hasten code reviews. So here are some of the current best practices and industry standards for coding.
Like with most crafts, you should make sure to know the tools of the trade. Take eslint, for instance. Now, eslint is an automated static code analyzer to check code for syntax error and problematic patterns. It is widespread and integrated in most modern IDE. If configured right and integrated well in their IDE, all errors would be highlighted on the spot and can be fixed while the code is being written. Another feature of eslint is to detect unused variables, imported libraries, and dead or unreachable code. By removing all that before publishing, you can easily clean up code.
And yet, many developers don't really use it in the local environment. They don’t run it or ignore the directives and rules given by the tool. As a result, easy-to-find problems are detected only later in the CI/CD, or never. And it ends up making everyone lose precious time when fixing super simple mistakes as early as possible would have spared them that.
Now, having no definitive naming convention or simply not following it properly can cause all kinds of problems. For example, we end up with variables like isActive, IsActive, is_Active that represent the same thing. Or a mix of cases like ReferenceForCard_key, can_AcceptCall, or all kinds of similar ugliness. Sometimes also, the name is not descriptive of what the method or variable does or contains. For example, a variable clientName contains the name of the client’s city. Or a method emailTemplate is actually sending an email. There are also cases where classes are not named properly. For instance, a class in a file called FindAllUser.dto.ts is called FindAllUser, instead of FindAllUserDto. As a consequence, naming conventions are not consistent across the whole project.
A great example and starting point for coding conventions is the AirBnb coding style guide that can be found here.
Code coupling is when two modules, or two layers are interdependent to a degree. Strong coupling creates many problems, most notably a code that is hard to follow and difficult to maintain.
A good example of coupling is when the default value of the pagination displayed on the frontend is coded in the data services layer. The data services then become difficult to reuse in a CRON, or for data analytics as they will include a pagination that is not needed.
Another good example is perhaps input validation that is often misplaced in the service/repository instead of the controller or the Entity/Model. The way we validate an input depends on the way we input data.
Moreover, these are often hard-coded instead of being in a validation class or using a validation engine like class-validator. Input transformation often suffers the same fate.
And maybe the best example of all is how a lot of developers still couple their API endpoints with a specific view on a frontend application. This creates a lot of problems as the endpoint cannot be reused for, let’s say, a view in a mobile application, or the admin panel. So now might be a good time to read the guiding principles of REST.
To resolve this, make sure to understand how a code is coupled and try to minimize coupling. And to decouple a code, understand the responsibilities of each layer and don’t step on the responsibilities of other layers. For example, a pagination is strictly linked to a call to GET /users by a client, so it’s the responsibility of the controller (or the Dto) to validate and transform this request. CRON jobs or Workers can now use the same services without worrying about having to deal with a paginated list of entries.
It’s important to note that bad architecture and design can cause many problems, most notably a lack of abstraction, duplicated code and tightly coupled components, and lack of flexibility and possibility for extending the code, which could very well lead to fear of making any changes. And most of the time, this happens when business logic is crammed into a place where they do not belong like Repositories or Controllers.
First off, and I can’t stress this enough, commenting code or having proper documentation is highly important, which I’ve covered already in this article.
Also, learn how to read documentation and spend serious time on it. Documentations are the bibles you want to learn from. No need to read the RFC or other ISO documents though. But you should really spend a considerable time on the Angular documentation before you declare yourself knowledgeable in that technology. YouTube videos can supplement this if you want to “see” a practical use case.
Lastly, learn to read documentation properly. Now that YouTube is a thing, some developers think they can learn anything without reading. The problem is, just like _The Lord of the Rings_ movies aren’t the full representation of the books, a video tutorial is not the full representation of what is in the documentation. Being a developer, knowing how to read and write code are fundamental skills you need to develop and hone.
Some people just feel like they always need to build a rocket or a gas factory. Perhaps it’s because it creates some sense of self-importance, or they just enjoy bragging about it later with friends. The problem is, over-complexification leads to a code that is hard to transfer to a different person and creates a lot of maintenance cost because we always have to “ask the other guy,” so to speak.
Also, over-complexification is related to a lack of comments and documentation. Often when the code is hard to explain, it's because it's too complex. Another cost of over-complexification is a complete disregard for what is essential such as security, coding convention, and naming. Often, a developer’s mind is so focused on trying to understand their own code and making it work that they completely forget good coding practices.
So always go for the simplest solutions because, more often than not, they’re the right ones. And also, when in doubt, trust your gut, take a pause, and think again.
Another common but avoidable mistake is having no validation on the input and no consideration for basic security such as XSS, CSRF, etc. Many developers still think that hiding a button on the interface will remove a functionality or avoid it being used. This is a terrible practice and is completely incorrect.
To resolve this, API endpoints need to be secured independently from the client. More importantly, approach your developments with a "Security First" mindset. And finally, familiarize yourself with common risks and learn how to avoid or minimize them. For this, you should read about the OWASP Top 10 Web Application Security Risks and learn how to avoid or reduce them.
Most of the time, the mistakes I see when reviewing code are fairly simple to fix. More often than not, though, they're the result of laziness, lack of knowledge, or just plain apathy or carelessness. Whichever the reason may be, these common mistakes, which can be avoided in the first place, may easily become an annoying bottleneck to your software development progress, or worse, a costly one. So better nip those bad habits in the bud before you get too far along in the development process… to spare you the unnecessary and totally unavoidable hair-pulling, keyboard-breaking, and screaming-to-the-screen episodes.
Eric has been working as a software engineer for more than 20 years. As a senior architect for Arcanys, he works closely with the developers to instill the habit of learning, clean coding, re-usability and testing with the goal of increasing the overall quality of the products delivered by the teams.