Rockin' good code, rockin' good times

Text

Here’s a thought experiment on removing complex conditionals from views. We haven’t done this fully at Reverb, so I’m just exploring it in this blog post to get some feedback.

My theory is that any usage of ==, &&, and || is problematic for our views (and actually, in all code outside the object in question), because it implies something that probably has a conceptual name, yet every person who has to read through the view code has to parse the conditional and understand what it means.

It’s more painful in views because those tend to be looked at by designers, which may not have intimate knowledge of the logic in question.

Let’s examine why these three things might be problematic:

Equality

payment.payment_type == 'credit_card'

The double equals implies knowledge of both sides of the equation. On the left hand side, if you’re asking about an object’s property, you assume knowledge of the types of values that property can take on. On the right side, you are assuming a particular value that may or may not be a true possible value. Here are steps we take to clean this up:

payment.payment_type == Payment::CREDIT_CARD
payment.payment_type?(Payment::CREDIT_CARD)
payment.credit_card?

The third variation is the most readable and hides internal implementation. You now have absolutely no knowledge about internal properties of the payment, nor of the possible values they can take.

Ands && Ors

There are a number of different types of boolean expressions that arise. Let’s first look at one where both parts of the conditional refer to the same object. In this case, we can condense it into one method on the same object:

Before:

auction.ended? && auction.loser?(current_user)

After:

auction.ended_and_user_lost?(current_user)

While the new method name is more verbose, I would venture to say that it’s more clear. Additionally, you are at less risk for someone copy-pasting bits of the original conditional and duplicating the logic. You have only one place that talks about losing an auction.

What about something that deals with two different objects? This is a common pattern at Reverb, where we often let admins and product owners do similar types of actions:

current_user.admin? || product.sold_by?(current_user)

How might we say this otherwise? How about:

product.can_administer?(current_user)

Or we could possibly use a permissions gem like CanCan.

What are you doing to simplify your views?

What other techniques are useful to make your code more readable, and more easy to work with by designers? Let us know!

blog comments powered by Disqus
Crafted in Chicago