Identification
A Little Bit of This, a Little Bit of That
If you work as a professional software developer, then you must have seen code like this before. Truth be told... we've all written code like this before:
if(user.role === "admin"
&& user.isActive
&& user.permissions.some(p => p === "edit")) {
// Do stuff
}
I know you've seen this before – and versions of this that are much worse and more complex!
These long lists of conditionals we find inside our if statements can be called verbose conditionals:
- They are hard to read.
- They are hard to understand once you've read them.
- They are hard to modify.
- They are hard to test (well... you can't really directly test them at all).
Misbehaving Conditionals
These lengthy conditionals often lead to software that will misbehave and not work the way you would want it to.
More often than not, when we find bugs in these places, we usually try to quickly add a fix, move on, and forget about it.
Then, another similar piece of code causes a bug the next week!
Finally, you realize it was actually the exact same logic copied and pasted – so you actually didn't fix the root issue!
The next section will help you tame these wild conditionals!
Combining Conditionals
Situation
In the following sections of this chapter, we are going to stick with the same example as the code snippet in the previous section. We'll improve it one step at a time.
Often, this is how you will approach refactoring: starting with the simplest refactoring. As your code becomes more complex or requires more TLC, then you can bring out the big guns.
Our scenario is a web application where we need to check some user permissions around editing a resource.
The Code
if(user.role === "admin"
&& user.active
&& user.permissions.some(p => p === "edit")) {
// Do stuff.
}
How can we make this more readable, understandable, less prone to bugs, and easier to maintain?
One of the quick wins you can get with these kinds of verbose conditionals is to start by moving each one into its own variable.
Let's start with this:
const isAdmin: boolean = user.role === "admin";
const userIsActive: boolean = user.active;
const userCanEdit: boolean = user.permissions.some(p => p === "edit");
Next, let's introduce a new variable that will combine the meaning of what we are trying to achieve in the first place, and use it in our if statement:
const activeAdminCanEdit: boolean = isAdmin && userIsActive && userCanEdit;
if (activeAdminCanEdit) {
// Do stuff.
}
Since we've named our variables very clearly and explicitly, this is much easier to understand.
Guideline
As a general guideline, try to make your if statements refer to the condition of one variable. If you are checking multiple variables, then consider combining them into a new one that will add more clarity and semantic meaning.
Extracting Methods from Conditionals
Consider the code we had in the previous section:
const isAdmin: boolean = user.role === "admin";
const userIsActive: boolean = user.active;
const userCanEdit: boolean = user.permissions.some(p => p === "edit");
const activeAdminCanEdit: boolean = isAdmin && userIsActive && userCanEdit;
if(activeAdminCanEdit) {
// Do stuff.
}
In some cases, the above code is fine.
In other cases, sometimes, you just get that feeling... something's not right.
What's Wrong Here?
Let's think through this code.
Is it possible that somewhere else in our application we will need to check whether:
- A user is an admin?
- A user is active?
- A user can edit certain resources?
The answer is... probably... yes.
Right now, is that logic able to be accessed by other parts of your application?
Nope.
Oh, yeah. There's another thing. All of these conditionals are performed in relation to your user.
The Fix
Why don't we take the logic from those conditionals and extract them as methods on our user class? Since these methods are performing logic that's about the user, it makes sense to give our user object ownership of these behaviors/states.
If we did that, we would get:
const activeAdminCanEdit: boolean = user.isAdmin()
&& user.isActive()
&& user.canEdit();
if(activeAdminCanEdit) {
// Do stuff.
}
That looks a bit better. But more importantly, you can share these pieces, which might be needed by other parts of your application!
Extracting Conditional Logic to Explicit Classes
In the previous section, we looked at extracting conditional logic as new methods on a user object.
Sometimes, though, we end up doing this refactoring over and over.
What we are left with is a User class that is filled with tons of these kinds of methods.
Note
This can be an issue that is best solved by actually modeling your classes using a domain-driven design approach.
While being beyond what this book covers, you might be interested in looking into the concept of bounded contexts (https://www.martinfowler.com/bliki/BoundedContext.html).
Let's Get Classy
Imagine that our User class looked something like this now:
class User {
isAdmin(): boolean { /* Code */ }
isActive(): boolean { /* Code */ }
canEdit(): boolean { /* Code */ }
isActiveAdmin(): boolean { /* Code */ }
isActiveAdminThatCanEdit(): boolean { /* Code */ }
// And dozens of more methods...
}
You can tell that this kind of stuff will get much harder to maintain over time since the number of methods is exploding!
Let's take the isActiveAdmin method, for example. What if we extracted this as an entirely new class? What would that look like?
class UserIsActiveAdmin {
private _user: User;
constructor(user: User) {
this._user = user;
}
public invoke(): boolean {
return this._user.isAdmin()
&& this._user.isActive();
}
}
SRP
In essence, we've just applied the Single Responsibility Principle, or SRP (https://deviq.com/single-responsibility-principle/), to our code.
Our User class doesn't have that method anymore, so it's smaller and therefore easier to understand and read.
The new class we've made deals with one specific responsibility and is therefore much more maintainable.
Using It
Using the new class would look like this:
const activeAdminCanEdit: boolean = new UserIsActiveAdmin(user).invoke()
&& user.canEdit();
if(activeAdminCanEdit) {
// Do stuff.
}
Now we get the benefits of:
- Having our logic sharable with other parts of our application
- Our User class not becoming a dumping ground for methods
- Our conditional logic still benefiting from the simplification of our code
Pipe Classes
In the last section, we created a new class, UserIsActiveAdmin, and ended up with this:
const activeAdminCanEdit: boolean = new UserIsActiveAdmin(user).invoke()
&& user.canEdit();
if(activeAdminCanEdit) {
// Do stuff.
}
Building these kinds of classes that deal with a single test or condition helps us to build lots of smaller pieces that we can later combine together for more complex scenarios.
This refactoring is useful, but I want to show you an even more flexible way to build these classes.
Your Classes Might Be Doing Too Much...
Looking at the code sample at the beginning of the chapter, what if we wanted to further combine all the logic into a single class?
We might call it UserIsActiveAdminAndCanEdit. Okay.
And that might be fine. But what if we have, let's say, eight different conditionals we need to check here?
Our class might have this name:
CheckOneCheckTwoCheckThreeCheckFourCheckFiveCheckSixCheckSevenCheckEight
Ouch. Probably not a good thing to do.
What can we do then?
Piping Our Logic
If your code base is getting to this point, then I suggest that you keep these individual classes limited to checking one or two things.
Next, we can create a common interface:
interface IPipeableCondition {
check(): boolean;
}
Each one of these classes will implement the interface, as in this example:
class UserIsActiveAdmin implements IPipeableCondition {
private _user: User;
constructor(user: User) {
this._user = user;
}
public check(): boolean {
return this._user.isAdmin()
&& this._user.isActive();
}
}
Imagine we had a good number of these classes. We could combine them together in a collection and pipe them all through a mechanism that makes sure all of them pass:
const conditions: IPipeableCondition[] = [
new UserIsActiveAdmin(user),
new UserCanEdit(user),
new UserIsNotBlacklisted(user),
new UserLivesInAvailableLocation(user)
];
const valid = conditions.every(p => p.check());
if(valid) {
// Do stuff.
}
We've created a pipe where we can specify what items are applicable and they will all automatically be combined at runtime.
This is usually not the go-to when designing your code. But it's good to know that it's an option – and when your code warrants it, it's a great way to simplify things.
Bonus Refactor
If we were going to use these pipes in multiple places in our app, then we would want to build another class that was responsible for the piping logic:
class ConditionsPipe {
private _conditions: IPipeableCondition[];
constructor(conditions: IPipeableCondition[]) {
this._conditions = conditions;
}
check(): boolean {
return this._conditions.every(p => p.check());
}
}
We would use it like this:
const pipe = new ConditionsPipe([
new UserIsActiveAdmin(user),
new UserCanEdit(user),
new UserIsNotBlacklisted(user),
new UserLivesInAvailableLocation(user)
]);
if(pipe.check()) {
// Do stuff.
}