Railsの太ったモデルをダイエットさせる方法について

こんにちは。メドピアのRuby(Rails)化をお手伝いしている@willnetです。最近はよくリファクタリングをしています。

今回は、最近僕がリファクタリングしている内容についてまとめようと思います。

メドピアではFat Model/Controllerを避けるために、rubocopの設定を利用しクラスの行数が300行以下になるよう制限しています*1。最近300行を超えるモデルが出てきたので、一部の処理を別のクラスに切り出し始めました。

このとき、Railsが提供している機能であるconcernsを利用すると楽に行数を減らすことができますが、それだとrubocopの指摘を回避できるという意味しかないので、なるべく委譲(composition)を利用して処理を別クラスに移していっています*2。

複数モデルにまたがる処理を切り出す

Railsアプリケーションを書いていると、複数のモデルを一度に使った処理を書きたいケースがあります。その場合は処理をどこに書くのが適切でしょうか？

多くのwebサービスではユーザが起点となる処理が多いので、結果としてUserモデルにメソッドが集まってしまいUserモデルがどんどんFatになります。数百行のレベルで済めば良い方で、千行を超えるUserモデルを持つプロジェクトも多いのではないでしょうか。行数が多いクラスは内容を理解するのが大変ですし、コードを修正した際の影響範囲もすぐにはわかりません。こうなるとコードに触れるのが苦痛になってきます。

これを解決するには、Active Recordを継承しないPORO*3なモデルを作成し、処理を委譲します。

例として、Userモデルに「ブログを投稿して友達に通知をする」メソッドを書いてみます。

class User < ApplicationRecord
  has_many :posts
  has_many :friendships
  has_many :friends, through: :friendships
  
  def create_post_with_notifications!(body)
    transaction do
      posts.create!(body: body)
      friends.each do |friend| 
        friend.notifications.create!("#{name}さんが投稿しました")
      end
    end
  end
end

これはUserモデルに置かれることの多い典型的なメソッドです。Userモデルが投稿と友達の関連元になるので、一見収まりがよく見えます。開発初期でUserモデルが小さいときはとくに問題になりませんが、開発が進むにつれこのようなメソッドが大量に存在するようになり、邪魔になってきます。

そこで、「ブログを投稿して友達に通知をする」という単一目的のクラスを作ってみます。

class PostWithNotifications
  def self.create!(creator:, body:)
    new(creator: creator, body: body).create!
  end
  
  def initialize(creator:, body:)
    @creator = creator
    @body = body
  end
  
  def create!
    ActiveRecord::Base.transaction do
      create_post!
      create_notifications!      
    end    
  end
  
  private
  
  attr_reader :creator, :body
  
  def create_post!
    creator.posts.create!(body: body)
  end
  
  def create_notifications!
    creator.friends.each { |friend| create_notification!(friend) }
  end
  
  def create_notification!(friend)
    friend.notifications.create!("#{creator.name}さんが投稿しました")
  end
end

処理の内容的には以前と変わっていません。ただ、メソッドを切り出したついでに多少リファクタリングしています。単一目的のクラスで管理することにより、メソッドを抽出するなどのリファクタリングがより簡単になりました。

PostWithNotificationsクラスを作ったことにより、メソッド呼び出しがuser.create_post_with_notifications!('投稿内容')からPostWithNotifications.create!(creator: user, body: '投稿内容')に変更されました。インターフェースを変えずに処理を切り出したい場合は、次のように、元のUser#create_post_with_notifications!から処理を委譲するようにするとよいでしょう。

class User < ApplicationRecord
  # 略
  
  def create_post_with_notifications!(body)
    PostWithNotifications.create!(creator: self, body: body)
  end
end

複数のレコードを扱う処理を切り出す

単一のモデルを取り扱う場合でも処理を書く場所に困る場合があります。それは複数のレコードを取り扱うケースです。複数のレコードを取り扱うために、モデルのクラスメソッドに処理を書く場合が頻繁に見られます。モデルが小さい場合はそれでも問題ないですが、先程と同様開発が進むにつれモデルの見通しを悪くする要因になります。そもそも、Active Recordは本来レコードとオブジェクトを一対一でマッピングするデザインパターンなので、複数のレコードを扱うクラスは別に用意するのが適切です。これも先程の例と同じくPOROを使うことで解決できます。

例として、複数のメッセージを既読にする処理を考えてみます。

class Message < ApplicationRecord
  enum status: %i[unread read]
  
  def self.read!(messages)
    messages.unread.each(&:read!)
  end

  # たくさんのメソッド
end

(この例だとMessage.read!メソッドは十分小さいのであまり切り出す必要性は感じないかもしれません。もっと長いメソッドを想像して読み替えてください)

このMessage.read!メソッドはメッセージを扱うので一見妥当な場所に存在するように感じます。しかし、複数レコードを取り扱うクラスを作り委譲させることで、より見通しがよくなるケースが多いです。

class Message::Collection
  def self.read!(messages)
    new(messages).read!
  end

  def initialize(messages)
    @messages = messages
  end

  def read!
    @messages.unread.each(&:read!)
  end
end

Message::Collectionというクラスを作りメソッドを切り出しました。これで開発が進みread!メソッドが複雑になったとしても、全体を容易に把握できるはずです。

単機能として切り出せる処理を切り出す

ここまでの内容に沿って複数モデル、複数レコードの処理をPOROに切り出したあとは、Active Recordのモデルに書かれている処理は基本的に自分のモデルに関わることだけになっているはずです。それでも行数が多く取扱いに困るときは機能ごとにPOROに切り出しましょう。

どういう機能を切り出すべきかはモデルごとに判断するしかないのですが、例えばバッチ用の処理などは共通して切り出しやすいです*4。

Active Recordのモデルに対してバッチ処理用のメソッドを生やすよりは、バッチ処理専用の小さいクラスを作ってしまった方が取扱いが楽なケースが多いです。

次のような、未読のメッセージがあったときにユーザにメールを送るようなメソッドがあるとします。

class User < ApplicationRecord
  # ...
  
  def self.notify_unread_messages
    User.active.find_each do |user|
      next unless user.messages.unread.exists?
      UserMailer.not_read_messages(user).deliver_now
    end
  end
end

これをバッチ用のクラスに移動させると次のようになります。

class UnreadMessagesNotification
  def self.notify(user: nil)
    new(user: user).notify
  end

  def initialize(user: nil)
    @user = user || User.active
  end

  def notify
    user.find_each do |user|
      next unless user.messages.unread.exists?
      UserMailer.not_read_messages(user).deliver_now
    end  
  end

  private

  attr_reader :user
end

メソッドを移動させたことでUserクラスからUser.notify_unread_messagesを削除できました。それだけではなく、メール送信の対象となるユーザを外部から注入できるようにしたことで、テストをしやすくなっています。

まとめ

POROを利用してモデルを小さく保つ方法について紹介しました。Railsの初心者は特に「モデルとはActive Recordのことだ」と考えがちです。その固定観念から抜け出すことで、可読性の高いアプリケーションを書く入り口に立てるのではないかと思います。この記事によって少しでも読みやすいRailsアプリケーションが増えることを願っています。

是非読者になってください(︎ ՞ਊ ՞)︎

メドピアでは一緒に働く仲間を募集しています。ご応募をお待ちしております！

■募集ポジションはこちら

https://medpeer.co.jp/recruit/entry/

■開発環境はこちら

https://medpeer.co.jp/recruit/workplace/development.html

*1:コントローラで300行は多すぎるので、いずれコントローラの場合は100行以下にしたいと思っています

*2:concernsについてはまた別の機会に書くかもしれません

*3:Plain Old Ruby Objectの略で、Active Recordなどを継承していないふつうのオブジェクトのことを指します

*4:この話の前提として、「基本的にバッチ用の処理はモデルに書き、rakeタスクはモデルのメソッドを呼び出すだけ」という慣習があります。これにより、処理内容がわかりやすく、かつテストが書きやすくなる効果があります