Code like this can slip through the cracks fairly easily because… it works. The tests are green, the code is clean!1

The problem is that we’re not being frugal. Checking whether the user is suspended requires hitting the database, making the first expression more expensive than the second. We’re out of order.

There are four possible combinations of whether a profile is archived and whether its user is suspended. How our code is currently written, here’s how Profile#active? would play out:

suspended?

archived?

active?

# of Queries

true

true

false

1

true

false

false

1

false

true

false

1

false

false

true

1

Simply changing the order of the conditional expressions produces the same results, only with fewer queries:

def active?
!archived? && !user.suspended?
end

suspended?

archived?

active?

# of Queries

true

true

false

0

true

false

false

1

false

true

false

0

false

false

true

1

In our example, the expression performing fewer queries is clearly less expensive to evaluate. In your own code, any number of factors will come into play, but it’s worth the effort to get your conditions in order.