Patterns that make your code less readable
Three years ago I published this article about bad practices often used by developers in Angular applications. If you have not read that article, it may be beneficial to read it first before this one.
Today, I want to focus on some other patterns that make our components/directives/services and other parts of our Angular apps less readable and harder to reason about. Without further ado, let’s get started!
Polluting the ngOnInit
ngOnInit may be the most important lifecycle hook in Angular components; it is used to initialize data, setup some listeners, create connections and so on. But sometimes this may make our lifecycle hook too overwhelming:
Take a look at this component. It does not have very many methods — actually, it has only two lifecycles. But the
ngOnInit method is, frankly speaking, terrifying. It subscribes to different form change events,
fromEvent streams, it also loads lots of data. It has 40 lines of code, but we have actually omitted the contents of the
subscribe callbacks; with them, it may easily be 100+ lines, which already goes against most soft guidelines. Also, we usually work with other methods and not the
ngOnInit, so we will need better access to the other methods, but now we would have to scroll through this whole mess to reach them (or close/reopen
ngOnInit every time we need to see it). Also, finding something inside the
ngOnInit method itself becomes harder because there are so many mixed concepts and tasks.
Now take a look at this revised version of the same component:
The logic of the component is the same, but how we arrange our code is different. Now the
ngOnInit method calls for three different methods to load the initial data from services, setup form change listeners and setup DOM event listeners (if needed). After this change, reading the component from scratch becomes easier (read the
ngOnInit — understand what it gets started at a glance, and — if you need implementation details — visit the corresponding methods). Finding the source of the bugs is also relatively easier: if form listeners don’t work correctly — go to
setupFormListeners and so on.
Don’t pollute your
ngOnInitmethod — split it into parts!
Writing useless directive selectors
Angular directives are a powerful tool which allows us to apply custom logic to different HTML elements. In doing so, we utilize css selectors, which actually gives us way more power than we care to realize. Here is an example: imagine a directive that checks if the corresponding element’s
formControl has errors and applies some style on it — let’s call it an
ErrorHighlightDirective. Now let’s say we give it an attribute selector, say
[errorHighlight]. It works fine, but we now have to find all form elements with
formControl attribute and put our
[errorHighlight] on them, which is a tedious task. But we can, of course use the
[formControl] directive attribute selector itself, so out directive will look like this:
Now our directive will automatically bind to all form controls in our module.
But the usage does not end there. Imagine we want to apply a shaky animation to all
formControls that have a
has-error class on them. We can easily write a directive and bind it using a class selector:
Use better selectors for your directives to avoid cluttering your HTML with unnecessary attributes
Logic inside a service constructor
Services are classes, and as such, have a
constructor, which is usually used to inject dependencies. But sometimes developers write some code/initialization logic inside it too. And sometimes this is not the best idea, and here’s why.
Imagine a service that creates and maintains a socket connection, sends data to the server in real time and dispatches even from the server. Here is a naive implementation:
This basic service creates a socket connection and handles interactions with it. Notice anything off?
The problem is, any time a new instance of this service is created, a new connection will open. And this just may not be the case we want!
Actually, lots of times an app will use a single socket connection, so when we use this service inside lazy loaded modules, we will get a new open connection. To avoid this, we need to remove the initialization logic from this constructor, and find another way to share the connection between lazy loaded modules. Also, we might want to have a method that allows us to reload the connection at will (essentially reopen it, for example if it unexpectedly closes):
Adding new state when you can derive it from the existing state
Every component has its state — a set of properties that hold data essential to render the UI. State is the most important logical part of our app, so handling it correctly has great benefits.
The state can be described as original and derived. Original state can be described as independent data that exists on itself — for example, the state of being signed in. Derived state is entirely dependent on some piece of original state — for example, a text notice that says ‘Please sign in’ if the user is logged out or ‘Sign out’ if the user is signed in. Essentially, we don’t need to hold that text value anywhere — anytime we need it, we can calculate it based on the authentication state. So this piece of code:
will turn into this:
As you can see, the
text property was derived state and was completely unnecessary. Removing it made out code easier to read and reason about.
Don’t create separate variables and properties to store derived state; calculate it whenever necessary instead
This one may seem a bit easy to spot, but when dealing with increasingly complex data, even the most experienced developers sometimes make this mistake, especially with RxJS streams. In this article I explore how this concept should be handled in RxJS Angular apps.
There are lots of mistakes one can do when writing an application with Angular. But some mistakes are very common and become patterns, that are reused and abused. Learning about the most common ones and how to avoid them can be very beneficial for our Angular apps.