Day 6 - Let's extract a compound conditional


#1

Week two! Let’s get after it.

Today’s task: extract a compound conditional.

This is best explained in a live-coding, video format. Fortunately, I have just the thing!

Please watch the first 5 minutes of this video: https://www.youtube.com/watch?v=0rsilnpU1DU&t=13s

The linked video is a sample of a course I created recently, and it contains my best explanation of how and why to extract compound conditionals.

Again, you only need to watch until 4:58.

For those of you that truly hate video, here’s a brief text version:

Compound conditionals look like this:

if foo && bar
  ...
end

Or

if foo || bar
  ...
end

Your language might write these slightly differently, but the essential bit is that you have a conditional whose truth depends on the truth of two components combined with a boolean logic operator.

I’ve found that extracting these compound conditionals into a named method almost always improves the code.

In abstract terms, that looks like this:

Before:

if foo && bar
  ...
end

After:

if higher_level_concept?
  ...
end

def higher_level_concept?
  foo && bar
end

And here’s a concrete example:

Before:

if user_created_account_today? && user_has_unconfirmed_email?
  prevent_user_from_posting
end

After:

if user_has_high_spam_risk?
  prevent_user_from_posting
end

private

def user_has_high_spam_risk?
  user_created_account_today? && user_has_unconfirmed_email?
end

I prefer the second version because it’s more explicit. It takes an implicit idea: “users with new accounts and unconfirmed emails are more likely to post spam” and makes it explicit in the code. This is usually a big win, since it means there is more information in the code, and less that exists only in developers’ heads.

Again, there’s even more useful info in the video https://www.youtube.com/watch?v=0rsilnpU1DU&t=13s, and it’s only 5 minutes, so I suggest watching it.

Today’s challenge: find a compound conditional in your code (just grep for && and ||) and try extracting it into a well-named method. Try to make the new method explain a little more about what the compound conditional means at a higher level.

If the new name is a big enough improvement, consider committing/merging/opening a PR. If not, no worries. Just revert and remember this lesson next time you reach for && or ||.


#2

Extracted a compound condition that appeared in two different places in the code :slight_smile:

In embedded/real-time code, sometimes adding one-line functions is too expensive (run-time).
So, when I encounter an unclear compound condition, I will try to use an inline function (if the compiler support it) or a MACRO (if it doesn’t) to clarify the meaning of the condition.
Especially, whenever I need to debug or expend a condition that I don’t understand on a first read and I need to spend some time understanding its purpose, I extract it.


#3

Good one. In vanilla Javascript it doesn’t look that neat but now my code is more explicit.

Used it to refactor some shortcut combinations, e.g.:

if(evt.shiftKey && evt.keyCode == 49)

was replaced by

if(isQuickAccessKeyPressed(evt))

I think it will be very useful if I gonna revisit old code of mine :sweat_smile:.


#4

Quick one for me.
I already have this good habit.
The only || and && I found were already in named method.


#5

This is a nice one! Readable code >> fewer lines.


#6

Really like this challenge. Unfortunately I wasn’t able to get my change completed in the 20 minutes. I found a method with a long conditional (8 different clauses and'd together), did the refactor, then discovered that the method didn’t have any unit tests in place. Started working on those, but wasn’t able to get it done in the 20 minutes.

Edit: so I went back later and completed them & put the whole shebang up for review. Was really interesting as it turned out the conditional I extracted had ~14 clauses and was duplicated verbatim in another spot, so I could just replace that with the method call. Happy side benefits!

One note: my language (Python) wasn’t so easy to find places to do this, as the boolean operators in Python are and and or not && and ||. Ended up doing a grep for and and restricting to only Python source files, and even then finding good candidates wasn’t as easy as one might think.


#7

I’ve been working on client proposals all day, rather than code. I’ll have a crack at the CQC on one of my side projects later. But, I have tried to avoid any complex conditional sentences in the prose I’ve written today :slight_smile:


#8

I’m often dealing with a large legacy code base, and the places this technique could apply are numerous. Even in the single controller I was in the middle of editing, there were ample options to pick from. Amusingly, the one I chose ended up being used 4 more times in the same controller, so I actually achieved a double win with this challenge.


#9

Done, a lot of double checks like :

if (body.data && body.data.username)

replaced by

if (haveUsername(data))

After a lot of refactorization in conditions I’ve continued cleaning a little bit more parts of the code. Great day.


#10

There were a lot of opportunities to make this change in the app I checked, but I took the easy way out.

I made a pretty common Rails app change of updating if thing.nil? || thing.empty? to be thing.blank?.

This is probably a little out of the spirit of the challenge, but I met my match for getting this done quickly with:

if new_ad_group.present?
  new_ad_group.enable! if feed.out_of_stock? && @campaign.paused? && !new_ad_group.unposted?
else
  # ...
end

#11

This is a complex one for me. I found 422 instances of compound conditionals on our app. Even thinking about going through them and extracting N methods for each is kinda mind boggling for me. I wonder if the big increase in methods that are only used in a single spot is worth it.

That being said, I did apply the pattern to a specific case. I got to a decent solution, but I believe this case could benefit with a policy. I’ll work on that a bit more.

It would be useful to see more examples of using this pattern in real world applications! Does anybody know of examples of this on open source code or something?


#12

@pedrogaspar, As with most things related to clean code, this should not be interpreted as a hard rule that must be applied on the whole codebase “or else it will be dirty”. It’s about readability, and I think the point of this exercise is to get familiar with such a refactoring. In real world you don’t need to apply this to every single compound conditional you can find, but where you find the statement to be difficult to understand quickly when reading the code.

Even if you wanted to apply this to or some other refactoring to a large codebase, you don’t need to do it all at the same time. Adopting a mindset to always be on the lookout for this kind of little improvements when working on existing code (and not being afraid to make those changes) will make your codebase cleaner over time as you work on it.


#13

Well said @cvuorinen. That’s something I really like about the QCQ, it helps building this mindset. It’s the starting point to make improving quality a habit.


#14

Thank you for that @cvuorinen :slight_smile: I guess it’s easy to be overwhelmed by thinking about applying this everywhere. And more importantly, as you say, this shouldn’t be a hard rule anyway.

Adopting a mindset to always be on the lookout for this kind of little improvements when working on existing code (and not being afraid to make those changes) will make your codebase cleaner over time as you work on it.

This is a great lesson for me ^ I’ve failed at adding other patterns to my tool set in the past for focusing too much on how to apply them across the codebase and thinking about explaining those changes to other people I work with… :sweat_smile:

Also, reading Day 7’s challenge, I was happy to see the “Finally, please don’t feel bad about any of the following” section at the bottom. And:

As always, this challenge is about showing up each day and taking a small step forward toward better code quality. Some days, you won’t improve the code itself, but your mind. Attempting each exercise will prime you to perform better on your next project or task.

Thanks @ben!


#15

I found a conditional which checked whether an index fell into a range, and extracted an easy-to-read function. It only gets used once as far as I can tell, but it’s still a readability win.


#16

Found lots of places where this was helpful. Even led to some more general opportunities to refactor, which I took. Glad to spend longer than 20 minutes on this one.


#17

This is a great challenge for the day.

This is also a code smell that is a really easy one to keep an eye out and spot in code-reviews. @pedrogaspar if you can’t implement it today on your codebase then use this as a reminder to watch for the next time you’re doing a code review and everyone will benefit :smiley:


#18

So today was doing a code review of my coworkers code (who is also doing the code quality challenge) and saw that he had used the extract compound conditional to good effect. Was cool to see it come up in daily work.


#19

This challenge had a nice double win hidden! I extracted out some common error handling in a class connection to SQS. I created the method

def sqs_request_error?(client)
client.errors.present? && sqs_response.nil?
end

But i also noticed that we were instantiating a connection to SQS over and over by calling the method that new’s up the connection! I was able to reduce SQS open connections by 4x!


#20

Great exercise for breaking long and complicated conditionals into more readable code!

As others have mentioned before, I didn’t try to apply this method to all my compound conditionals. Only to the ones that made sense.

As one alternative to breaking the conditionals into private methods, when I don’t feel like creating them (e.g. there are already many private methods in the current class), I’ll use intermediary properly named variables to achieve the same goal.