Refactoring

A few years ago, I inherited a reasonably simple system based on Node.js that had many basic functionalities, such as authentication and database connection baked in. Since I had much bigger problems to solve at that point, and everything seemed to be working just fine, I ignored those things, treating them as black boxes. Finally, after a while, when I found some spare time to refactor the codebase, I had the opportunity to review and modify it according to my best practices, keeping all the knowledge I have in mind. It is great to have a legacy system that works, but in a dynamic startup environment, it is better to make things easy to change, so a decoupled system that is easy to read, understand, and change is fundamental.

The first thing I experimented with was hiding the implementation details. We use several services, including Firebase, Stripe, Slack, AWS, Voucherify and a couple of others, that leak into the production code directly, making a nicely shaped big ball of mud. So the first thing I did was to push the implementation details to an integration package. Only this package has a direct dependency on the services themselves. It exposes the API in a form that is specific to our app and tailored for its needs, hiding things that are irrelevant inside the app-specific methods. Often, it is a one-to-one mapping, but I believe it is worth the effort. We can’t control third-party library updates that might break the API, so it’s better to update just one place rather than searching the entire app for usage. This Integrations package is designed specifically to call services like Firebase, but most domain features are not service-dependent. They care about authentication, push notifications, sending emails, checking vouchers, and other related tasks. To prevent the service name from leaking to the app domain, I created another abstraction layer that aggregates the functionalities, not the services. So the push-notifications service is responsible for sending the messages. If I want to send a PN from the app, I call it, and only the corresponding method knows that it is calling Firebase to do it. The client doesn’t care as long as the message is delivered. At this level, I store the application logs collected by our logging system. The logging system itself is done in the same abstract way. The services call the log method, and only the logger knows that it needs to contact the Loggly integration, which hides the third-party library details. This pattern is used for both customer-facing functionality and development tools.

Next, I looked at the environments that we use. I’m unsure if we need both development and production, or if we could manage with production alone, supplemented by feature flags and cookie usage to force services like Stripe to use the testing environment. We already have this setup, and I don’t plan to change it now. We need the config files and a mechanism to select the appropriate credentials for each environment. Previously, a function was required to resolve the correct environment using OS-provided parameters. However, since this is simple data, it doesn’t need to be wrapped in a function. Instead, I resolve the config at app startup into a constant and use it to select the appropriate database and either production or testing accounts in the third-party services. This is a pretty small change, but conceptually it makes much more sense to initialise it and access it as simple data eagerly. A crucial aspect is to prevent the config from leaking to every part of the app. It probably makes sense only to use it in the integrations.

Lastly, I looked closer at the authentication and authorisation techniques used in the app. This is a crucial part of the app that could get very complicated. Fortunately, we delegated the most complex part of this to Firebase, and only simple token management is left in the app. We also have some workaround for the integration testing that hardcodes a user to be logged in without using the Bearer, which makes it a bit more complicated. For a very long time, I avoided touching it, as it seemed fragile and a bit tangled. It appears to be code written to work, but never refactored for readability. So firstly, trying to understand what is going on, I started to pull the functions out, grouping functionalities and hiding the complexity behind meaningful method names. This helped me to realise what is going on and where. It also helped to recognise duplicated code that could be reused.

Looking back on the refactor, I wish I had started earlier, running fast to catch up with aggressive timelines despite limited resources. I ignored the big ball of mud for way too long. It was working, of course, but without proper understanding and having a clear structure, we would run into massive troubles with the scale. Hopefully, I have some time before the scale comes, and hopefully, one day we will get the problem and the clean, easy-to-understand and maintain code will help us overcome it.