![Search icon](https://hackernoon.imgix.net/search-new.png?w=19&h=19)
Picture the scene: You have a critical bugfix to make in your payments system, and the stakeholder is nervously watching on from the corner of the room asking helpful things like “How close are we to the fix?”.
You start by looking at some code in get-payment-methods.ts
, the top-level entry-point of an API route.
// get-payment-methods.ts
const getPaymentMethods = async (req) => {
const paymentMethods = await loadPaymentMethodsForCustomer(req.customerId);
return res.send(paymentMethods);
}
That looks reasonable, you think to yourself.
You need have a look at what loadPaymentMethodsForCustomer()
does. Ok, let's go.
CMD-click.
// services/payment-methods.ts
const loadPaymentMethodsForCustomer = async (customerId) => {
try {
const traceId = uuid();
const customerPaymentMethods = getPaymentMethodsOrThrow(customerId, traceId);
return customerPaymentMethods.map(someConversionFunction);
} catch(err) {
return errorHandler(err);
}
}
Hmm. Now you need to look at getPaymentMethodsOrThrow()
.
CMD-click. CMD-click. CMD-click.
Deeper and deeper we go into the tangled web of nested functions. Sunlight can’t penetrate this deep into the codebase, and long ago you lost track of why you are here.
Some time later, you arrive at Ground Zero, where the Actual Thing Is Happening. In this case, the code is making a call to Stripe’s API to fetch a customer’s saved payment methods.
// repositories/stripe.ts
await stripe.paymentMethods.list({ customer: stripeCustomerId });
Finally!
Sure, you found the bottom of this particular set of nested functions. But now you are trying to understand what the code does to the data, for example calculation, transformation, and mapping.
Typically with nested functions, something is happening to the data at almost every level in the tree. For example, one function might do a try/catch
. Another might convert the array to just return the id
field of every payment method. Another might supplement that information with a second API call. And even after you have followed the loop all the way to the bottom of the tree and back up again, there might be an entire second set of nested functions that comes afterwards.
In short, it is nearly impossible to build a mental picture of the sequence of events. You might know in your heart that somewhere there is a logical flow comprising ten things that happen one after another, but it’s going to take you three days and six whiteboard pens to figure it out.
It’s time to make the code change! You steel yourself. Let’s go.
if(paymentMethod.expiry < new Date()) {
throw new Error("Oh dang, try a new card");
}
Ok. Let’s run the tests.
npm test
Instant fire 🔥🔥🔥. Explosions. 600 tests failed. What happened?!
The problem with deeply nesting functions is that the dependency graph between different parts of your codebase quickly becomes orders of magnitude more complex than it needs to be.
Let’s say that Function A calls Function B, which calls Function C. Function X also calls Function B, which calls Function C. Function Y calls Function Z, which in turn calls Function C, too.
So when you changed Function C, you broke functions, A, B, X, Y, and Z.
You spend three hours trying to understand exactly how all of those functions work. You give up, and just copy/paste Function C into a new area to avoid breaking all the other stuff. Sad face emoji 😔
Let’s say that you have Function A, which calls Function B.
const functionA = async () => {
// complicated things here
const data = await functionB();
// more complicated things here
}
You have some complicated logic in Function A. You decide that it is deserving of some nice, thorough unit testing.
The problem here is that to test Function A, you have two options:
Test Function A and Function B
Use mocks to bypass Function B
The problem with Option 1 is that now our unit tests are not really unit tests. We only really wanted to test the pure logic, and now we’re doing stuff with async calls, which requires a lot more effort.
The problem with Option 2 is that mocks are nasty and reduce confidence in your tests. They are a useful tool when absolutely necessary, but the moment you change functionB
, your tests for functionA
are now at risk of giving you false positives.
Neither option is great, and before you know it, you are part of the Testing Is Hard And I Don’t Like It Club. Very sad face emoji 😪
I like to use something that I call the Orchestrator/Actions Pattern.
An Action can be almost anything. For example, an action might:
The most important characteristic of an action is that it strictly follows the Single Responsibility Principle. In simple terms, it should only do one of the above. Either fetch data, or transform it. Not both.
An Orchestrator calls many actions in a sequence.
An example Orchestrator might look like this:
const exampleOrchestrator = async () => {
const initialData = await getInitialData();
const calculation = runCalculation(initialData);
const otherData = await getOtherData(calculation);
const combinedData = combine({ initialData, otherData });
return combinedData;
}
The most important characteristic of an orchestrator is that you can clearly see the sequence of events. Sure, you might have some simple logic such as if
statements to determine which actions to call, but as long as it is easy to read, you have done a good job.
It isn’t always possible in reality to write code without any nesting. Sometimes, nesting can be helpful. But if you can generally strive to write your code using the Orchestrator/Actions Pattern, your code will be:
I hope this is helpful. Let me know your thoughts in the comments!