DRYing up Rails Scopes and Instance Methods

This is a small tip about DRYness, which is generally held to be a Good Thing.

It is also a good Rails practice to make generous use of scopes.

And I notice that in some cases it is difficult to reconcile both of these fine practices.

As a fairly trivial example, consider a scope that is implemented as:

scope :active_non_defaults, -> {
  where(active: true, default: false)
}

Fundamentally it defines a set of database records/instances with logic that is executed at the database layer.

It is used to return to your system an appropriate set of records, and for each of the instances based on these records you can infer that the condition "is this active and non-default?" is true.

If you had a single instance that you had retrieved through some other method, perhaps a #find, and then wanted to test whether it met a active_non_default? condition, how would you do that?

Possible you would implement a question mark method on the instance:

def active_non_defaults?
  active? && !default?
end

So this is fine, but is the code still DRY? Arguably not, as the system now implements the same logic twice, once to find a set of records in the database and once to test whether an instance is a member of that set.

This happens to me all the time, and here is one way around it.

# A scope. Nothing to see here, same as above
scope :active_non_defaults, -> {
  where(active: true, default: false)
}

# A new scope. Hello, what's going on here?
scope :is_this?, ->(thing) {
  raise "Was expecting a Thing" unless thing.is_a? Thing

  where(id: thing.id)
end

# An instance method that uses both of them.
def active_non_default?
  self.class.active_non_defaults.is_this?(self).exists?
end

Or you might go a step further with:

scope :includes_this?, ->(thing) {
  raise "Was expecting a Thing" unless thing.is_a? Thing

  where(id: thing.id).exists?
end

def active_non_default?
  self.class.active_non_defaults.includes_this?(self)
end

What does this do?

It executes the query defined by the scope, adding in a new condition to say "just for this one row", and tacks on the ActiveRecord method #exists? that transforms the logic into a test of whether such a row exists.

Or in other words, "is this one row a member of the set defined by the scope's query, yes or no?".

It's not a great example, as you usually don't want to be executing SQL if you can avoid it and here it is easy to avoid, but if the scope is complex (perhaps referencing other scopes, and perhaps running a query anyway) then it starts to make more sense:

scope :deletable, -> {
  not_active.
    older_then_six_months.
    not_flagged_for_undeletable.
    has_no_outstanding_issues.
    approved_by_the_lawyers.
    has_a_backup.
    not_subject_to_deletion_laws.
    etc
}

def deletable?
  self.class.deletable.includes_this?(self)
end

Of course in some cases this will perform worse than a skilfully handcrafted instance method, but because of the cleverness of database query optimisers this can be very efficient under the right circumstances.

Consider that the query executed by our refactored instance method tells the database that it is not interested in a large set of data. The query optimiser knows that the SQL generated by where(id: thing.id), things.id = 354 perhaps, is a primary key lookup of a single row, and it finds that to be a very attractive execution starting point.

A good quality query optimiser is going to check that condition first (and we can infer that it is going to be true), and then look through all of the other conditions in the SQL and apply them in the order most likely to return the result set as quickly as possible.

So if the scope not_active is a quick test on the row, and approved_by_the_lawyers requires a join to a view with three unions and four correlated subqueries, then it's going to check the not_active condition first. Maybe it gets lucky with that and never has to check with the lawyers, and now you have a 1.2ms test.

It is a long standing principle in database performance optimisation that combining multiple queries into a single query will perform better than the multiple queries executed individually.

You might have implemented your instance method more conventionally and in the most efficient order, of course, with something like:

def deletable?
  not_active? &&
    older_then_six_months? &&
    not_flagged_for_undeletable? &&
    has_no_outstanding_issues? &&
    approved_by_the_lawyers? &&
    has_a_backup? &&
    not_subject_to_deletion_laws? &&
    etc?
end

... but if three of those instance methods are implemented with queries then you will end up running all three of them on a deletable instance, instead of one.

But of course this is not always the case and nor can this be used for an instance that is not yet created, or which has changed since it was last saved to the database.

So I will be using this with discretion and consideration of many factors, and offer it to you as an option for your consideration.