Refactoring conditionals

Refactoring conditionals

Conditionals are supposed to add intelligence to code, but they can be a nightmare to understand.

Why refactor conditionals

Our targets when refactoring conditionals are:

Making it easy to understand the conditions that we are checking: conditions can be hard to understand if we check for specific values that are meaningfully related to the concept they try to evaluate. See this example:

The condition is fairly easy to describe in human terms: select documents that have too long names. But its code expression is really hard to understand. Let’s simplify this a bit, extracting the conditional expression to a method with a meaningful name.

Reduce the cognitive complexity when following the execution flow through their branches. When you read a conditional structure you need to keep track of the main flow. If you introduce conditionals inside the branch of another conditional you are creating new breaking points that need to be tracked. At a certain level, you will overflow the ability of your working memory to handle all that tracking.

You can easily spot this by looking at the indentation levels of the code. The more indented it is, the more difficult it to understand and the easier it for bugs to appear.

You can manage this with several refactors. The most simple and easy to apply is to extract code in branches to methods. That opens new refactoring opportunities inside the private methods.

Conditions with meaning

You don’t always need conditions

We use conditionals to control the execution flow of a program: go this way if a condition is met. If not, go this other. But, sometimes, we can avoid that. Let’s look at this example:

We don’t need the flow control part here, we are served with the response from the object. Also, it’s pretty confusing that you return false in the true leg of the conditional.

To be precise, we can simply return the negation of that response:

This kind of refactor helps to avoid some potential points of failure. With the original code, you have to ensure no less than three things:

• The method invoked provides the correct value

• The conditional expression evaluates to the correct value

• Each leg of the conditional returns the correct value

Now, you only have to check one thing:

• The method invoked provides the correct value

Ternary operator instead of if/then

The ternary operator is pretty convenient under certain conditions. You should use it carefully because it can bring more problems than solutions. But it is perfect for this situation:

When we have two possible ways of calculating a value depending on a simple condition, the ternary operator is perfectly fine:

The conditions to use the ternary operator are:

• We need to choose between two ways of calculating something

• We don’t nest ternary operators.

The benefit here is that we state clearly that we are calculating something (the $event that we want to send in some next step).

If it’s complicated, extract a method

The ternary operator works pretty fine in the simple situations exposed before. But if the calculation is more complex or can’t be reduced to clean construction, you’d better extract a method and hide the conditional inside of it:

Replace type checking with type safety

Take a look at this code:

This code reveals a big bad smell. If you need to check for type to perform a task, then you have a design problem. Probably, you are trying to ensure that the Visit has a related Patient, so you can do something with it.

Does it make sense that a visit has no patient-related? If not, you probably should have to fail with an exception. In fact, you should ensure that when instantiating the Visit object. Anyway, the Visit::getVisitPatient method should be typed to state that the expected outcome is a VisitPatient. By doing that, you can enclose the code block in a try/catch to manage the situation.

Visit::getVisitPatient() guarantees that it only will return a VisitPatient. Otherwise, something is wrong.

If not having a VisitPatient is fine for Visit, then you should use nullable return type. This way, instead of checking for type, you will check for object existence, making code cleaner:

You can do this because getVisitPatient guarantees that it will return a VisitPatient or a null, keeping consumers safe about the type, but having to manage the situation of empty value. So you simply need to check that $visitPatient has a value.

Also, remember that this example could reveal a violation of the Tell, don’t ask principle or Demeter’s law. It is possible that code related to VisitPatient can be happily encapsulated in Visit. You can take a look at this previous article on the topic.

Give combined conditions a meaning

Sometimes combined conditions represent a concept that is not explicitly put in code. Let’s see a pretty simple example:

This line of code makes you think about what it is actually checking for. This line is in the context of an SMS notification service, so it makes sense to think about the idea of a patient can be notified via SMS or not. This can be achieved if both two conditions are met:

• The patient has a phone number (that can receive SMS)

• The patient chose to allow being notified with SMS

So, instead of asking about these two things to the same object, we can encapsulate both questions into only one, inside VisitPatient, because both conditions belong to the object:

If we don’t have a better place to move the combined conditions, we can simply create a private method. This is the proper way when we are using values from different origins.

Managing the flow

Conditionals are control flow structures and we use them to take different actions when certain requirements are met. We’ve been talking about the conditions, but now we will focus on their branches.

We can have different problems with the branches of conditional expressions:

• Too large branches: too much code in the body of a branch makes it easy to lose track of the context.

• Unbalanced branches: one of the branches is comparatively larger than the other, making the latter invisible.

• Nested conditionals: conditions inside conditions are especially difficult to understand and are a perfect place for intricated bugs to hide.

Too large branches

The best approach is to extract branches to their own method with a descriptive name for the abstraction level. This will make it easier to understand the overall flow, and you always be able to dig into details if needed.

Look at this code. The branch is remarkably long:

We move all the code in the branch to a new method, taking care of managing how we return the data:

Now, the main flow is far easier to read. We can dive into details by jumping into the extracted method. We even have a chance to extract this code to a collaborator in the future if we need to reuse this particular piece.

Unbalanced branches

When one of the legs of a conditional structure is a oneliner and the other is huge, it is good practice to invert the conditional, so the shortest branch appears in the first place. This avoids missing it:

It’s much better to extract both branches to methods with a significant name. This way you stress the fact that two flows are depending on the province variable having value or not.

You should consider eliminating the else branch. Sometimes you won’t need it. For example, because you can perform an early return.

Nested conditionals

Nested conditionals are a source of frustration and pain. They are difficult to follow and unsafe to modify when needed. How can we avoid them?

First of all, try to ensure that nesting is needed. In the following example, we can see that the inner conditional should be moved to another place. We can guess that by putting next, things that are related to each other. For example, the calculation of validSince.

The value of validSince can be calculated before, instead of waiting until being in the conditional. And we can introduce the null coalesce operator to simplify the assignment using the first value among all possible that it’s not null.

Our first approach will be to extract the main branches into private methods. Then, you repeat this extraction iteratively so all methods have a maximum of one indentation level in each one. This opens opportunities for further refactors that are pretty difficult, or impossible, in the nested structure. In fact, part of nested structure complexities relies on the fact that you have to carefully manage the execution flow to make sure you don’t overwrite variables or introduce some other bugs.

Let’s take a look at this code extracted from the famous GildedRose kata:

This is only half of the code and has a lot of nesting.

For this example, we extract the legs to their methods. Note that this kata deserves an object-oriented approach, but I want to illustrate a different point this time:

We can see that in the extracted methods the body is inside a conditional. We can apply a return early refactor that will reduce the nesting. We need to invert the conditional expressions first. As you can see, the readability has increased a lot.

We can apply return early again in the increaseQuality method:

At this point, we can spot some remaining code duplication that we could refactor. It can be argued that the nesting is there yet. Nevertheless, the takeaway here is that with pretty simple steps we have structured the code so it is much more readable and easier to understand.

Final words

Conditionals can be tricky to manage, but we can improve their readability a lot by using simple refactoring techniques and thinking twice about why we need them.

Of course, we could go further by applying a good object-oriented design that can even avoid the need for conditional structures. But that should be the topic for another post. In the meantime, you can watch this talk by Sandi Metz about how and why to achieve this using the Gilded Rose example.

Keep an eye on our blog for more tips about refactoring and testing to keep your code healthy.

Do you enjoy the article? Follow our profile and visit our other channels: Docplanner.tech website, Facebook, Twitter, and LinkedIn.