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

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:

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.

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 ❤️

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!