How to refactor some horrible code

David Torres
Feb 10 · 5 min read
Pyramids in the desert
Pyramids in the desert
Pyramids are beautiful, if-else pyramids are not. Photo by Steffen Gundermann on Unsplash

Recently, I had to refactor some code that was in a rough state:

I didn’t actually had to… but look at it, I “had to” (╯°□°)╯︵ ┻━┻

The above logic determines the URL for endpoints and it depends on multiple things: the service you’re using, if it’s rendered on the server or the client, if it’s a production or a test environment, etc.

One of the reasons a piece of code can become so messy is when we forget that duplication is far cheaper than the wrong abstraction.

The good news is that you can apply a couple of simple techniques to simplify the nested if-else statements. The bad news is that this piece of code is crucial to the application as all requests are calling it. Oh, and it wasn’t tested either! 😬

So, here is how I went about refactoring it…

1. Give that code 100% test coverage

I was not tasked with refactoring this code, I made that call. But I didn’t know how much time it was going to take (time I should be using to deliver something else).

Refactors per se are not always justified. However, it’s difficult to imagine somebody complaining about increasing test coverage, especially if it’s covering such a crucial piece of functionality. So I decided to start with this, not only to give me confidence for the refactor, but also because by the time I finished the testing I would have a better idea of how difficult the refactor would be and I could make a go/no-go call.

Nowadays most of us practice TDD in Gousto, but this particular section of the codebase was from a few years ago and the importance and state of it discouraged me and others to refactor it in the past.

The primary benefit of TDD is the ability to refactor without fear, and without cost — Robert C. Martin (Uncle Bob)

I also wanted to make sure that the coverage was 100%, so I used the Jest flag --coverage which gave me the following output:

What is this thing I’m feeling? Oh yes, it’s my confidence raising!

2. Extract functionality

Now that we have more confidence thanks to our tests, we can start breaking the code apart. Let’s start from the beginning. We see that protocol is assigned different values depending on the service, environment, client/server side… and then added to the rest of the url at the end of the function.

const url = `${protocol}://${domain}`

So, we can abstract the code that determines the protocol into its own function and just call it once:

We can do the same thing we did with getProtocol() with the rest of the parts of the URL. The more functionality you extract the more the if-else hell gets simplified, and the easier it becomes for the remaining ones to be extracted. Therefore, my advice here is to start with the least complex as that will make the remaining easier.

Extracting these functions is not complicated, but that is because I translated the if-else nightmare to them. So we don’t have a big mess, but we have a few little messes. Unacceptable. Let’s take care of it.

3. Simplify

Apart from the separation of concerns, the advantage of extracting the different parts of the url into different functions is that the conditionals can be simplified further. Before, some parts of the URL were limiting the simplification as they required multiple conditions to be met. Now, they are separated so we don’t have to worry about that.

You could do this simplification just having a good eye for it… or you can use a truth table to help.

In the case of getProtocol(), a sort of truth table would look like this:

but it can be simplified a bit (‘-’ means the value does not matter):

We only have 2 possible values: http and https, so we can take the one that is in fewer rows (https), check for the conditions for it and return the other (http) otherwise:

This is already better than what we had in the previous section. But we can still do more! Checking the first two conditions you could realise that you only get https when we are not in the server side. So we can write a first condition for http and get rid of isServerSide in the rest of the function:

Still, we can do better. We could merge the second and third condition now that they are smaller:

But hold on… that condition is a bit silly isn’t it? If the service is webclient the condition is true. Else… it goes to the second part of the OR, why do we need to check that service is not webclient then? If we are in the right side of the OR then we are sure it is not webclient. The end result looks pretty neat, especially compared with the original one in section 2. Extract functionality:

Result

Thanks to the extraction of functionality we ended up with an endpoint() function which is a pleasure to write.

It is so simple that it should be a crime

The get functions of the code above are small enough to be understood without melting your brain. Not all of them are as simple as I would like, but they are much better than the original code. You can see here how the file ended up looking like.

Finally, you should always try to follow the rule: Leave your code better than you found it. Don’t do it for the praising, although it is appreciated ❤️

Image for post
Image for post
GitHub comment in the PR
Image for post
Image for post
A team mate making my day

Now it is your turn, do you have any ideas how to make it even simpler (without changing how it is used in the rest of the application)? Would you have approached the refactor differently? If so, let me know in the comments!

Gousto Engineering & Data Science

Gousto Engineering Blog

Medium is an open platform where 170 million readers come to find insightful and dynamic thinking. Here, expert and undiscovered voices alike dive into the heart of any topic and bring new ideas to the surface. Learn more

Follow the writers, publications, and topics that matter to you, and you’ll see them on your homepage and in your inbox. Explore

If you have a story to tell, knowledge to share, or a perspective to offer — welcome home. It’s easy and free to post your thinking on any topic. Write on Medium

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store