In one of my previous post I talked about “Clean Frontend Architecture” and principles that should be followed in order to positively influence the maintainability and scalability of frontend applications.
This time I would like to show the most common “Code Smells” that I have come across in Angular.
So, what are “Code Smells“ and why are they so bad?
Well, I would say the best definition of “Code Smells” is:
The total count of WTFs during a code review.
One might also measure the Code Quality with this procedure. A reference for that would be the book “Clean Code“ by Robert C. Martin, where the Code Quality is defined by the WTFs/Minute.
But why are “Code Smells“ bad? Because they negatively influence the maintainability and/or performance of any application.
I have seen so many “Code Smells” and bad practices that I have to categorize them. But how can this be done? One approach would be to categorize them by their negative impact on the performance. In this post I will categorize them as follows:
- Bundle Size
- Runtime Performance
- Maintainability
In the next sections I would like to point out the top 5 code smells of each category.
Bundle Size
In order to better understand the influence of bad practices on the bundle size one might analyze the application structure with the help of the “webpack-bundle-analyzer“.
What’s the “webpack-bundle-analyzer“?
This tool graphically illustrates the bundle size of each module, component and library that is used within the application. The following picture represents such a depiction.
The following mistakes are my personal top 5:
1.) No gzip configured (server or client side)
2.) Importing other styles in component styles
3.) Too large images or/and the use of SVGs in CSS
4.) No lazy loading of modules
5.) Too many third-party-libraries
The first mistake that I would like to address is that no compression is used. This massively influences the overall bundle size. So, please use compression on server or client side, simply use gzip.
Next to compression, what has the most influence on the bundle size?
Definitely “component styles”. In Angular all the HTML- and Typescript-Code will be translated into Javascript. Since Ivy, each component has a link to the dependencies which makes the resulting Javascript “tree-shakeable“. In different terms, all the unused code will be removed from the bundle. However, the resulting css is not that optimized.
So, importing other styles within component styles has a negative impact on the bundle size. Imagine that each component imports the following scss-File. Each component would import the Material-Theming, which will blow up the bundle size.
Another bad practice would be the use of SVGs in CSS. Such a use would create JavaScript that will blow up the bundle size. Related to SVGs is the use of large images. There are lots of applications out there that neither use a CDN to optimize images nor other tools to decrease the image size.
As the downloading of JavaScript is single-threaded, splitting the bundle into several small bundles is a good idea. This can be achieved by lazy loading modules. However, if everything is downloaded at once this could take a while.
And last but not least the use of too many third party libraries could also increase the bundle size. Don’t get me wrong. I am a big fan of third-party-libraries. However, sometimes tasks can be achieved without them. Especially “lodash“ is one of the libraries that can be easily replaced by build-in ES6-features.
Runtime Performance
There are a lot of bad practices that negatively influence the runtime performance. Almost all of them have to do with the Change Detection (CD). In Angular each application is represented as a component tree and this tree will be traversed each time an event is triggered. With the default CD strategy the templates will be re-rendered a lot, with the CD strategy OnPush not so much.
1.) Default Change Detection Strategy instead of OnPush
2.) Binding methods in the template
3.) No trackBy in *ngFor
4.) The use of detectChanges instead of markForCheck
5.) Multiple subscriptions and memory leaks due to open subscriptions
Almost all applications that I have seen use too many method bindings in the template. So, what should be used instead? Pipes, if possible. Or observables with the async pipe in the template. Moreover, I have seen the abuse of getters quite often. Instead of returning a value, the getters contain logic. In the following picture array-manipulations are used to retrieve the usernames and the selectedProduct. This must be avoided at all cost.
The next mistake is the use of *ngFor without a trackBy. Why is this a bad practice? Because during the rendering the items are checked for changes (if they are dirty). Without trackBy all items will be re-rendered each time the CD runs. With trackBy items that did not change will not be re-rendered.
Another common mistake is the use of detectChanges instead of markForCheck. I mean, there are use case for both of them, however sometimes markForCheck is the better choice. The picture below shows the difference between those two methods (Hint: all components that are marked as dirty will be re-rendered).
And the last mistake happened quite often in the past, open subscriptions. However, these days this mistake occurs seldomly. Multiple subscriptions on the other hand are still a big problem. Due to that the logic will be triggered multiple times. In case of simple data this would not be a big problem, however if there are transformations or API-requests that will be triggered on subscription, then this will definitely lead to a bad runtime performance.
Maintainability
Well, if it could only be that easy to always use best practices, then the code would stay maintainable, applications would be more robust and the project managers would be happy. However, most times mistakes simply happen. These are the top 5 mistakes that I have encountered so far.
1.) Handling imports — No module resolution
2.) Multiple responsibilities in Components
3.) Utility-Functions in Services
4.) Nested subscriptions
5.) Mutable instead of immutable
The first mistake is about shortening the imports. Relative paths are not easy to read and definitely not that maintainable. Imagine, for example, that some models are moved to another module. Then each import has to be changed. However, with module resolution this would not be the case anymore. And the readability and maintainability is way better.
Another common mistake is that components do not have one responsibility but multiple ones. Maybe there are too many services injected or the component logic is way to complex. For all such cases components should be split into multiple ones.
The third mistake that I have come across quite often is that util-functions are not implemented as static methods within some class, but are implemented as methods in a service. Why is this bad? Util-functions are easier to test. Services are not always the best choice.
The next mistake that I would like to discuss are nested subscriptions. Observables can be easily mapped and the result subscribed to. This is more readable and also not that error-prone. That is the case because multiple subscriptions could lead to memory leaks due to unhandled observables/subscriptions.
And last but not least stay immutable. There is no need to directly modify objects or arrays. The only exception is performance. So, if there is some critical code that has to be very performant, then big arrays can be modified directly. But please don’t make this a habit.
Build Angular Apps with reusable components, just like Lego
Bit’s open-source tool help 250,000+ devs to build apps with components.
Turn any UI, feature, or page into a reusable component — and share it across your applications. It’s easier to collaborate and build faster.
Split apps into components to make app development easier, and enjoy the best experience for the workflows you want:
→ Micro-Frontends
→ Design System
→ Code-Sharing and reuse
→ Monorepo
Summing Up
I have given a definition of “Code Smells“
The total count of WTFs during a code review
Moreover, I have shown common Code Smells in Angular categorized by Bundle size, Runtime Performance and Maintainability.