Code golfing review: how to make composable Active Record queries

A quick backstory first. This week, Matt Swanson requested a roasting of this code he posted:

I'm generally not a roast person, but I am a complete sucker for code golfing 🏌️

In this post, I'll walk you through where I ended up and my thinking behind it. We'll then look at this in a broader context and what I think you can take away for your Rails apps.

My idiomatic Rails take

I replied with my take and here's the code in full:

module Room::Metrics
  def self.included(klass) = klass.has_many(:counters, class_name: "Room::Counter")
end

class Room::Counter < ApplicationRecord
  belongs_to :room
  belongs_to :organization, default: → { room.organization }
  belongs_to :participant_session
  belongs_to :block, optional: true

  scope :current, -> { where(occured_on: Date.current) }

  def self.increment_block_ids(kind, block_ids, **) = block_ids.each do |block_id, by|
    increment(kind, block_id:, by:, **)
  end

  def self.increment(kind, participant_session:, block_id: nil, by: 0)
    find_or_create_by!(kind:, participant_session:, block_id:).tap { _1.increment(by:) }
  end

  def increment(by: 0)
    super :value, by.to_i, touch: true
  end
end

class Rooms::BlockViewingsController < Rooms::BaseController
  require_feature :rooms

  def update
    Current.participant_session.humanize!

    @room.counters.current.with_options participant_session: Current.participant_session do |current|
      current.increment :viewing_time, params[:view_time]
      current.increment_block_ids :block_viewing_time, params[:block_ids]
    end

    head :ok
  end
end

Quick Tip: sometimes I prefer to read and compare code without reading the underlying explanations to help me form my own thoughts and see what parts I prefer more. You can learn to pick out strengths and weaknesses that way.

Ok, let's look at some quick wins and then the larger pieces.

Quick Wins: require_feature

I pretended a require_feature class macro existed. It's a before_action wrapper and I've written about how to clean up your Rails controllers with them.

Quick Wins: reinterpreting increment

Since we've trying to establish a concept of a Room::Counter, e.g. a counter-like object that's specifically for a room we can reinterpret Active Record's increment method in this context.

Specifically, that we don't expect outside callers to be able to increment anything besides our actual value attribute.

I'm also trimming away the ! since it seems overused in the original code and then not that communicative.

  def increment(by: 0)
    super :value, by.to_i, touch: true
  end

Composing Active Record queries when using find_or_create_by!

Let's look a bit closer at the find_or_create_by! in increment_counter!:

def increment_counter!(kind:, for:, block_id: nil, by: 0)
  counter = counters.find_or_create_by!(
    organization: organization,
    kind: kind,
    participant_session: binding.local_variable_get(:for),
    block_id: block_id,
    occurred_on: Time.current.to_date
  )
  # …
end

Tip: we could write Date.current since that's a shorter version of Time.current.to_date.

This method is the final call to inject parameters before we execute a query via find_or_create_by!.

Now, by embedding all these parameters in the final call we've got less flexibility in our system. If we need to inject more parameters we'll have to update this method. Sometimes that kind of notice is what we want, other times it can make one change cascade far and wide.

Here, if we instead follow Active Record's method chaining design, we can gain more choice while arriving at simpler composable code.

Specifically, if we do counters.where(occurred_on: Date.current).find_or_create_by!, Rails includes occurred_on: in the underlying find call or create if nothing's found.

Now we can extract a scope :current, -> { where(occurred_on: Date.current) too. This means we can have increment_counter! do less, and have more choice on the call site.

Composable Active Record queries: when concerns stop us

Except, we can't! Choosing to place this logic in the Rooms::Metric concern, means Room will have increment_counter!, requiring us to call counters.current.find_or_create_by! within increment_counter!.

Sometimes, this inflexibility is what we what and placing a finalizer method in a concern is the right call. In this case, we can gain more by moving this logic back into Room::Counter as a class method.

Since Rails lets you call class methods on a scope, we can now do room.counters.current.increment :viewing_time, ….

class Room::Counter < ApplicationRecord
  def self.increment(kind, participant_session:, block_id: nil, by: 0)
    find_or_create_by!(kind:, participant_session:, block_id:).tap { _1.increment(by:) }
  end
end

I went for just increment here since a) I don't think ! is helpful communication here and b) the room.counters.increment already explains that we're interacting with a counter.

As a side-benefit we've now get a class level increment that mirrors the instance level increment method. So things are starting to feel a little more consistent too.

I'm omitting organization here, since it simplifies what this method needs to know about. I'm assuming that the counter organization is always the same as its room's organization, so it'll work.

We can get the organization here in increment with proxy_association.owner.organization. The owner is the room from room.counters in this case.

What details do our Active Record methods need?

Let's look at increment again. We're requiring both a participant_session: and block_id::

  def self.increment(kind, participant_session:, block_id: nil, by: 0)
    find_or_create_by!(kind:, participant_session:, block_id:).tap { _1.increment(by:) }
  end

However, like we've talked about, we could supply those via where chaining methods, like room.counters.current.where(participant_session: Current.participant_session).

Part of the trick when writing more idiomatic Rails code is making a decision on what to pin down in our methods and what to keep loose to leverage Active Record's chaining queries' flexibility.

We could consider writing increment like this:

  def self.increment(kind, by: 0, **)
    find_or_create_by!(kind:, **).tap { _1.increment(by:) }
  end

Now we can pass block: in addition to block_id:. Because Active Record supports passing other Active Records directly, like accounts.find_by(user: User.first) and accounts.find_by(user_id: 1).

The deal when writing methods that wrap Active Record API is that the required arguments just has to be filled in when we start executing the query, but they don't all have to come at the end.

Finally, cleaning up our controller's story

With all the changes we've made we can revisit our controller and rewrite it to:

class Rooms::BlockViewingsController < Rooms::BaseController
  require_feature :rooms

  def update
    Current.participant_session.humanize!

    @room.counters.current.with_options participant_session: Current.participant_session do |current|
      current.increment :viewing_time, by: params[:view_time]

      params[:block_ids].each do |block_id, ms_viewed|
        current.increment :block_viewing_time, block_id:, by: ms_viewed
      end
    end

    head :ok
  end
end

This is a more than perfectly fine place to stop.

But now that we've tilled the soil by moving the increment method to Room::Counter we can clearly spot an extra opportunity. We can encode and highlight one extra concept to our counters, namely that incrementing "blocks" seems to be a core part of them.

So in comes increment_block_ids:

  def self.increment_block_ids(kind, block_ids, **) = block_ids.each do |block_id, by|
    increment(kind, block_id:, by:, **)
  end

  def self.increment(kind, participant_session:, block_id: nil, by: 0)
    find_or_create_by!(kind:, participant_session:, block_id:).tap { _1.increment(by:) }
  end

I'm placing it directly above increment to keep them together. I also prefer keeping higher abstraction methods above whatever building block methods they're using.

Now our controller can clearly and concisely communicate the similarities and differences between the two calls:

    @room.counters.current.with_options participant_session: Current.participant_session do |current|
      current.increment :viewing_time, params[:view_time]
      current.increment_block_ids :block_viewing_time, params[:block_ids]
    end

This goes to a programming idiom I use all the time: "make similar things more similar and different things more different". So here we've grouped and collected the similar things while highlighting the different part via the _block_ids suffix. I keep meaning to write more about this. One day!

Wrap-up

Thanks for reading! I hope this wasn't too dense. Writing to try to capture a lot of the internal nuance, as I'm weighing options between code choices, are proving to be tough. I hope this makes sense and gives you some jumping off points to explore and experiment more.

You're more than welcome to ask questions, I'll try to reply.