Tale of a Refactor

January 29, 2024

Taming Codebase Complexity Through Abstraction, the Costs and Benefits

Introduction

Software engineering is equal part art and equal part science. To be more precise: the higher level language we use the more likely the the primary consumer of your code is going to be another human rather than a machine. Your fellow engineer or yourself six months from now. Therefore the goal is not only to write performant code and make the machine happy. You need to write easily readable, maintainable, pluggable etc code. The code you author should have the least amount of what the heck is going on moments. Sounds easy?

If you've been in the software engineering business for a while, you know it's not the case.

Software codebases are complex systems where the parts of the system interact with each other all the time. The more parts we have, the more they interact with each other, the more complex the system becomes.

This is the defining characteristic of a complex system: the interactions between the units matter way more than the properties of the unit. This is the basic thesis of system theory and the exact opposite of the so-called reductionist idea. What is the reductionist idea you might ask? It is saying that a system is nothing more than the sum of its building blocks.

Reductionism in its strongest form holds that all the rest of reality, from organisms to a couple in love on the banks of the Seine, is ultimately nothing but particles or strings in motion. - Reinventing the Sacred - Stuart A. Kauffman

I already bored the dear reader with my philosophical rambling. Let me get back on track and tie the above train of thought to software engineering.

Think about the different types of tests we have: unit, integration and e2e. That is by no means an exhaustive list but sufficient to prove my point. How? If software systems are not complex, why do we need integration and/or e2e tests? Wouldn't unit tests alone would lead us to utopia and a bug-free world?

We know it doesn't. We can have 100% unit test coverage yet our product can be unusable. Interactions matter way more than the properties of the given unit. One of the reasons software systems get complex.

So come closer and let me tell you a tale of untying such a Gordian knot.

Story of a payment system

No matter what you are selling the end of the funnel is always the same: you are providing some value and you need to collect money for it. Thus failing to do so makes your product useless. It's a crucial part of any application.

That payment part is the protagonist of our story.

Fortunately, by the time of writing (2024) we have a lot of service providers who abstract all the tedious and risky logic away. So implementing a checkout/payment page is easy right? I mean how hard it can be? Teenagers are building apps with a checkout page in less than a day after they come home from school. ChatGPT can build one for you too. So where is the catch?

How is complexity introduced then? As with always, it creeps up on you, step by step.

Simple beginnings

Our journey starts when we only have one payment method to implement: Credit card. You only have one payment method you have to care about.

alt text here

Everything is simple. We don't have to take into account any different route the consumers can take. They can enter their credit card details, click on the Place Order button and that is it. After the loading spinner (and the underlying network request) is finished, we can see a confirmation page. End of story.

export const PaymentFormSimple = () => { React.useEffect(() => { initCreditCard(); return () => { tearDownCreditCard(); }; }, []); return ( <form onsubmit={() => { handleCreditCard(); }} > <label>CreditCard</label> {creditCardInput()} <ErrorComponent /> <PlaceOrderButton /> </form> ); };

A potential code snippet for such an application might look like this. What happens here? Let me quickly give some explanation.

We have a useEffect where we fire any housekeeping code that takes place while we mount and unmount our component.

handleCreditCard function is responsible for whatever happens after the Place Order button is clicked. Transformation of the given data, any network request to perform to handle payment authorization, inventory checks, interacting with order management systems, your analytics, any side effects, you name it.

Last but not least you have the UI elements there. Your radio button, your inputs, the place order button and your error element in the case of an unhappy path.

The main idea is the readability. Readability is achieved by how concise the logic and UI elements are. There is no distraction developers need to parse. Why or how it gets onto the screen or what business logic it executes. It’s simple.

const handleCreditCard = async () => { await authorizeCreditCard(); await capturePayment(); await reserveInventory(); await createOrderManagementEntry(); await finalizePayment(); };

Take the handleCreditCard function as an example. Even though it does a lot, it reads well. We can add more logic and more lines, yet it wouldn't impact its readability (much). No complex condition checks exist to conclude which branch we need to execute. No permutations to keep track of.

One of the main reasons for its simplicity is you don't have to keep much information in your head. There is no jumping back and forth and context-switching in your head.

Starting to get complicated

As our sprint is filled with chunky feature requests. We are busy implementing more and more methods and our application is getting bulkier.

alt text here
alt text here
alt text here

Our app changed a lot. Many of these changes are subtle yet changes they are.

For example:

  • Have a look at the description next to AfterPay. It is an optional text only shown while AfterPay is selected.
  • Our Place Order button when PayPal is selected has a different style and text.
  • The absence of the body (three input elements: card number, CVV and expiry date) when we select any method except for the credit card.
alt text here
alt text here
  • The different error messages based on methods.

The most important change however is what happens when we click on Place Order, our submission logic. If we look at the flow of AfterPay: it's nothing like a normal credit card journey.

We click on Place Order and as a next step, we will be redirected to a different domain. We ended up on AfterPay's domain. There we need to enter our wallet details and authorize the transaction (happy path). On successful authorization, AfterPay will redirect to our domain.

This flow (which involves a handoff at some point) is pretty common, it's not specific to AfterPay. My agenda here is to demonstrate how diverse submission logic can be based on different methods.

Complexity continues: different vendors

Unfortunately, our journey into the depths of branches does not stop here. We all know that feature requests rarely stop landing in our backlog.

At least if we implement a payment method, say credit card for a region, we are all set. For the next region that has credit card, we are all good, Right? Not quite.

There are many different implementations provided by many different vendors. They are sufficiently distinct to make it hard to create a coherent abstraction. Stripe, Adyen, Razorpay, name it. They all implement credit cards differently.

Possibilities are endless so are the permutations we can end up with. The safest way therefore is to treat a different implementation for the same method as if it were a completely different method.

Feature flags

We crushed all of our tickets. Every payment method is implemented. We have appropriate vendors for every region: multiple credit cards, PayPal, etc... All good right? Same old: not quite so.

Assume you got a ticket and the requirement is to slightly modify the card flow. Requirement is to have an middle step where we can have one final step before committing.

This is the credit card that we already implemented, yet the flow is unlike what we used to have. Now we have a review step.

Assume one region wants to have an intermediary step before we charge the users' card. That in turn creates a new piece of UI, a completely new branch in the submission handler flow and an optional reviewComplete handler (what happens when we click the Place Order button on the new screen).

Let's see what we need to do if we want to have every scenario covered in our PaymentForm.

Branches, Branches everywhere

export const PaymentForm = () => { React.useEffect(() => { switch (selectedPayment) { case PaymentType.PayPal: { switch (region) { case Region.USA: { paypalInitUS(); break; } case Region.UK: { paypalInitUK(); break; } default: break; } } case PaymentType.CreditCard: { initCreditCard(); break; } case PaymentType.InjectedCreditCard: { initInjectedCreditCard(); break; } default: break; } return () => { switch (selectedPayment) { case PaymentType.PayPal: switch (region) { case Region.USA: return tearDownPayPalUS(); case Region.UK: return tearDownPayPalUK(); default: return; } case PaymentType.CreditCard: { tearDownCreditCard(); break; } case PaymentType.InjectedCreditCard: tearDownInjectedCreditCard(); break; default: return; } }; }, []); const inputs = () => { switch (selectedPayment) { case PaymentType.CreditCard: switch (region) { case Region.USA: return paypalInputsUS(); case Region.UK: return paypalInputUK(); default: return; } case PaymentType.InjectedCreditCard: return injectedCreditCardInput(); case PaymentType.CreditCard: return creditCardInput(); default: return; } }; const submitHandler = () => { switch (selectedPayment) { case PaymentType.PayPal: switch (region) { case Region.USA: return paypalSubmitHandlerUS(); case Region.UK: return paypalSubmitHandlerUK(); default: return; } case PaymentType.InjectedCreditCard: return handleInjectedCreditCard(); case PaymentType.CreditCard: return handleCreditCard(); default: return; } }; const error = () => { switch (selectedPayment) { case PaymentType.PayPal: return <PaypalErrorComp />; case PaymentType.InjectedCreditCard: return <CreditErrorComp />; case PaymentType.CreditCard: return <CreditErrorComp />; default: return; } }; return ( <form onsubmit={submitHandler}> {paymentMethods.map((method) => { return ( <> <label>{method}</label> {inputs.map()} </> ); })} {error()} </form> ); };

Even if there were no commentary, the code snippet would do all the talking. This is just not right. It's quite mental to keep track of what is happening in this pseudo-code, and a possible real production code could be even more complex.

So what is essentially the issue here? The way I would phrase it: one possible user story is spread across many switch statements. (You can replace switch statements with if/else or if or object mappings. The main point here is the branching.)

For example:

The place order user journey (the purpose of the whole payment page we can argue, which ends up with us collecting money for our product/service) using credit cards is split across many different declarations. Therefore to understand the logic, one requires a lot of context switches, sort of saving in our brain what happens in one switch, putting that to the stack in our brain, and playing this push-pop game all day long.

The issue is, that our brain stack is not permanent. We forget things. The context switch and cognitive load make parsing logic like this very hard. Very hard if not impossible to comprehend. Imagine being a new developer. You are being onboarded to the project and you are assigned to put out a fire. Good luck with that. But even if you have the context of the code, most likely that knowledge will fade within half a year. Your future self is going to be just as confused as any newcomer.

alt text here

We are pissed. Rightly so. We added all the features, but it left us with a very noisy (British politeness) codebase.

Managing complexity through abstraction

We've seen how a relatively simple page (refer to the "Simple Beginnings" section above) can get complex. Incremental additions can increase complexity in a non-linear fashion.

What can we do about it? Every codebase is different so there is now one size fits all solution. Design patterns emerged over the decades to reduce this sort of complexity.

There is one commonality, however. There is always a pattern or patterns that emerge from the problem (chaos). Based on that pattern there is a possibility to create an abstraction. Creating an abstraction requires writing more code, therefore increasing complexity. Again.

So the question is this: is creating an abstraction worth it? Does the complexity and the code the abstraction creates offer enough value that justify the extra work and maintenance?

If the answer is yes, go for it.

Can we see a pattern here?

We identified the main pain point as this.

A potential user journey is stretched between multiple if statements (or switch....). This leaves us with a big cognitive load, hard maintenance and all the similar problems. That is our goal here: to reduce that maintenance burden and make our code more human-friendly.

alt text here
alt text here

Have a look at the picture. Highlighted are the parts of the page/application changed through the course of this blog post. For each of them when we introduced a new branch. Some UI elements. Our submission logic also changed.

Extract the pattern

export const getPaymentPrimitives = (paymentType: PaymentType) => { switch (paymentType) { case PaymentType.PayPal: { return { Label: () => <span>PayPal</span>, Button: PayPalButton, Body: null, Review: null, }; } case PaymentType.CreditCard: { return { Label: () => <span>CreditCard</span>, Button: CreditCardButton, Body: CreditCardBody, Review: CreditCardReview, }; } default: { throw new Error("Not a valid type"); } } };

What if flip our logic on its head: have one switch statement and define an internal API that we return for each branch? We create an API that needs to return elements like Button, Body etc.

What if would have to return elements roughly to what we have in our UI. How will that help us?

const CreditCardBody = () => { const config = useConfig(); React.useEffect(() => { // Can handle side effect local to the given component. }, []); const { onSubmission, onReviewConfirmed } = React.useContext( CheckoutWrapperContext ); onSubmission.current = async () => { // Normal API logic to save the payment info. // Can handle error handling and reporting. }; onReviewConfirmed.current = async () => { if (config.features.review) { // Handle logic if/when review is expected. } }; return ( <> <input /> <input /> <input /> <CreditCardErrorComponent /> </> ); // Render JSX. };

Let's zoom into the <CreditCardBody /> component. A <CreditCardBody /> component now only deals with what you have to render for the credit card. Inside that component, every line of code deals with the credit card-related business logic.

  • The useEffect runs credit card-related code.
  • You return JSX only for credit card.
  • We have something called onSubmission.current (which I'll explain why we need context in a minute). Now it's enough to say this function will include the submission logic only for the credit card. You define your handling logic here.

In other words, if you want to know about how credit card works you have to open this file, and nothing else.

This file has everything related to credit cards, nothing less nothing more. I want to stress the last sentence:

Nothing less and nothing more.

Nothing less, meaning you don't have to chase other files, put them onto your brain stack, parse them, or go back and forth. Nothing is "stretched" between branches, far easier to parse the code.

Nothing more, meaning code is not leaking from other parts of the application. The abstraction is concise enough not to incorporate code that does not belong here. That is a big deal when we need to extend our codebase (open-closed principle).

That Context API

export const PaymentPage = () => { const { Label, Button, Body, Review } = getPaymentPrimitives(currentPayment); return ( <CheckoutProvider> {config.payments.map((payment) => { return ( <div key={payment}> <div> <input type="radio" value={payment} /> <span>{payment}</span> <Label /> </div> {props.currentPayment === payment && props.children && ( <div> <Body /> </div> )} </div> ); })} <Button /> {Review && <Review />} </CheckoutProvider> ); }; const CheckoutWrapperContext = React.createContext<{ onSubmission: React.MutableRefObject<() => void>; onReviewConfirmed: React.MutableRefObject<() => void>; loading: boolean; setLoading: React.Dispatch<React.SetStateAction<boolean>>; // other metadata }>(null as any); export const CheckoutProvider = (props) => { const onSubmission = React.useRef<() => void>(() => null); const onReviewConfirmed = React.useRef<() => void>(() => null); const [loading, setLoading] = React.useState(false); return ( <CheckoutWrapperContext.Provider value={{ onSubmission, onReviewConfirmed, loading, setLoading }} > {props.children} </CheckoutWrapperContext.Provider> ); };

Our UI elements in the PaymentPage.tsx are going to be siblings most likely. Therefore sharing some common properties and their setters is easiest when using the Context API. That CheckoutWrapperContext just provides us with the convenience to pass our metadata easily between components. I.e. we might need to set the loading state in our <Body /> but the spinner will be visible in the <Button /> component.

onSubmission.current is just a way to set the submission handler without triggering another UI diffing cycle. It is also part of our API now. Just as we need to return certain UI elements from the getPaymentPrimitives function, we need to set and use our submission logic via our context. These two (getPaymentPrimitives and CheckoutWrapperContext) together are our API surface area.

Benefits of this approach

What is the value proposition of this approach? We saw how the UI is rendered, and what our API is (what we need to return and what metadata we can work with). We also touched upon how the <CreditCardBody /> is now only concerned with what happens with the credit card.

But what is the real benefit of this approach? First, let's explicitly state what our API surface area is:

  • <Button /> a mandatory component
  • <Body /> an optional component
  • <Description /> an optional component
  • <Review /> an optional component
  • onSubmissionHandler
  • onReviewConfirmed handler
  • various props (like loading)
// PaymentFormSimple - Our form when we only had credit export const PaymentFormSimple = () => { React.useEffect(() => { initCreditCard(); return () => { tearDownCreditCard(); }; }, []); return ( <form onsubmit={() => { handleCreditCard(); }} > <label>CreditCard</label> {creditCardInput()} <ErrorComponent /> <PlaceOrderButton /> </form> ); }; // CreditCardBody const CreditCardBody = () => { const config = useConfig(); React.useEffect(() => { // Can handle side effect local to the given component. }, []); const { onSubmission, onReviewConfirmed } = React.useContext( CheckoutWrapperContext ); onSubmission.current = async () => { // Normal API logic to save the payment info. // Can handle error handling and reporting. }; onReviewConfirmed.current = async () => { if (config.features.review) { // Handle logic if/when review is expected. } }; return ( <> <input /> <input /> <input /> <CreditCardErrorComponent /> </> ); // Render JSX. }; // CreditCardButton const CreditCardButton = () => { const { onSubmission } = React.useContext(CheckoutWrapperContext); return ( <button onClick={() => { onSubmission.current(); }} > Place order </button> ); };

Time to compare our new API with our original PaymentFormSimple, where we only had credit.

Our two forms are pretty similar and they read all the same. All the elements and handlers sit in the form component and we can understand them easily. Both the PaymentFormSimple (original, one method only), <CreditCardBody /> and the <Button /> (new refactored solution) deal with only the credit card-related business logic. Both of them are devoid of any serious branching thus it is easy to comprehend.

We sort of cherry-picked the good parts from our simple state. There is no need to juggle between codes. Of course, now the element is abstracted away and conditionally rendered. But that is part of our API. The main point is this: in both the SimplePayment Form and the refactored Form we can be sure: if we click on the element rendered (i.e.<Body />) we will be taken to a component where we only have to face the logic for the given method, nothing less, nothing more!

Build a new method

How can we benefit from the above approach? We talked about what it would take an onboarding developer to deal with a fire. Let's take another hypothetical scenario: simulate creating a new payment method.

Let's implement AliPay.

export const getPaymentPrimitives = (paymentType: PaymentType) => { switch (paymentType) { case PaymentType.AliPay: { return { Label: () => <span>AliPay 🌐</span>, Button: () => { const { onSubmission } = React.useContext(CheckoutWrapperContext); return ( <button onClick={() => onSubmission.current()}>Place order</button> ); }, Body: () => { const { onSubmission } = React.useContext(CheckoutWrapperContext); React.useEffect(() => { // Load the script. const script = document.createElement("script"); script.src = "<https://ali.script.com>"; script.onload = () => { // Render the UI. window.Ali.render("placeholder"); window.Ali.setSubmissionHandler = (amount, metadata) => { // do the usual things. }; }; document.body.appendChild(script); G; }, []); onSubmission.current = () => { window.Ali.submit(); }; return <div id="placeholder" />; }, Review: null, }; } default: { throw new Error("Not a valid type"); } } };

If we know the API we created it's pretty easy to add a new method. In this example, we inlined our components, for more visibility. All the elements deal with the corresponding logic. The <Label /> and <Button /> are pretty obvious: they return the required UI elements for the given method.

The <Label /> component is merely responsible for returning an optional description next to our radio button. Remember the case of AfterPay. The <Button /> component renders what we going to see at the bottom of the form. We just add an onClick handler that will call our onSubmission.current. (Which is part of our context API, and it can be set elsewhere).

Speaking of which, let's see how we set that.

The bulk of the work is done in the <Body /> component. In this particular example, we assume Ali Pay uses a third-party script. Therefore we initialise it in a useEffect. We assign our onSubmission handler to call the Ali Pay script's submit method. We assume this got attached to the window object due to our useEffect. We also render a placeholder so our downloaded script can take over and render any DOM elements as its child.

What we can see from this pattern is that only one switch statement is needed. Writing and later reading or modifying this logic is kind of easy. At least it's easy to grasp what the code does. No branching, no jumping in the code. Concise abstraction over a few units that have well-defined responsibilities.

What is the price?

Before wrapping up our story I'd like to talk about the tradeoffs this solution created. Nothing is free in this world. Creating an abstraction always comes with a price. In this case, we created an interface that consists roughly of the elements described here.

That interface is not common knowledge. It's not part of any library API. That means:

  • any onboarding developer has to learn it
  • it needs to be documented
  • needs to be maintained
  • extended, modified and altered if needs be
  • it adds limitations on how to build anything that depends on it
alt text here

Abstraction is not free. Especially the first point is huge. It creates friction as it's rare that only a handful of people will create software that would scale to enterprise levels. Therefore the burden of the abstraction must be lower than the burden of the current implementation.

What did we learn?

We solved some problems. As codebases grow complexity will rise. It's inevitable. We can't eradicate it, only tame it. What we discussed here is a story that worked for us in a React codebase at a dedicated point in time. Will it work for every case? Can you copy-paste it and apply it blindly? Of course not.

Then what is the value for you my dear reader?

Resist the urge to create immature and early abstractions. Only create them where there is some real pain. Abstractions are expensive. More importantly: they require patterns. Recognising the patterns takes time.

Related Posts

Rust vs Go: Which Is Right For My Team?

August 29, 2024
In recent years, the shift away from dynamic, high-level programming languages back towards statically typed languages with low-level operating system access has been gaining momentum as engineers seek to more effectively solve problems with scaling and reliability. Demands on our infrastructure and devices are increasing every day and downtime seems to lurk around every corner.

Timing and Talking: Lessons from Working with Screen Readers

August 15, 2024
Often as digital product designers we consider our job to be entirely tied to the visual medium, and spend most of our time gauging the proportions of elements on the screen, what kinds of tactile actions a user can or should be able to perform, and how best to optimize our product’s intended function, look and feel.

Please Don’t Make Me Guess: A Design System Guide for Designers and Developers

August 6, 2024
Vague feedback often adds a lot of frustration to the software creation process. By reducing the number of these blind spots, we could collectively stop thinking, “please don’t make me guess”.