Rails の scope をあまり使わない方がいい理由

1. Rails の scope とは

Rails には scope という便利機能があります。
https://railsguides.jp/active_record_querying.html#%E3%82%B9%E3%82%B3%E3%83%BC%E3%83%97

例えば

class Article < ApplicationRecord
  scope :draft, -> { where(published_at: nil) }
end

のように定義しておけば、下書きの一覧を表示するページで

Article.draft.order(updated_at: :desc)

などと呼び出せるようになります。

これはコードを短く書く上で有用な方法のひとつです。
何をしたいのかわかりやすいコードとなることでしょう……本来は！！

2. ダメンズメーカーの scope

しかし、実際のところそうはなりません。
scope の定義をまとめている箇所が、以下のようなダメなコードの養殖場となっているケースをよく見ます。

class User < ApplicationRecord
  has_many :articles
  has_and_belongs_to_many :roles, join_table: :users_roles

  scope :latest_users, -> { order(created_at: :desc).first(5) }
  scope :rank1_writer, -> { find_by(id: Article.pluck(:user_id).group_by(&:to_i).values.max_by(&:size).first) }
  scope :admin, -> { where(deleted_at: nil).select{ |user| user.roles.map(&:key).select{ |key| %w(super_admin admin).include?(key) } } }
  scope :active_writers, -> {
    latest_articles = Article.order(:updated_at).last(10) + Article.order(:created_at).last(10)
    where(id: latest_articles.pluck(:user_id))
  }
end

Rails のコードを複数社で見たことがある人は「ああ〜わかる〜」とうなずいてくれるかもしれません。

私がコードを書くときに意識していることとして、
「たとえ、既存コードのコピペでコードのほとんどを書くエンジニアが引き継ぐことになっても、ある程度回るようにしたい」
という思いがあるので、その観点でいくと、
「scope って使うべきではないのでは？」 と感じていますので、その理由をここではまとめます。

3. scope を使うべきでない理由

3-1. 1行で書こうとしてしまう

上の例では複数行で書いている例も混ぜましたが、基本、scope は1行で書くことが多いです。
それにならって他の人も1行で書こうとします。
そうして出来上がるのが以下のような、複雑な処理です。

scope :rank1_writer, -> { find_by(id: Article.pluck(:user_id).group_by(&:to_i).values.max_by(&:size).first) }
scope :admin, -> { where(deleted_at: nil).select{ |user| user.roles.map(&:key).select{ |key| %w(super_admin admin).include?(key) } } }

一見しただけでは何をしているのかわかりにくいです。
よく言われることですが、Ruby は短くコードが書けてしまうぶん、
可読性の低い1行の処理を書けてしまうので注意が必要です。

scope で書かれているメソッドは、何のクラスが返ってくるのか
一見してわからないことが大変よくあります。

3-2. コメントが書かれないことが多い

たとえ「メソッドの上部に yard に従った説明コメントを書くこと」という開発ルールがある会社でも、
scope にはコメントが書かれていないことがよくあります。
scope がメソッドであると意識されていないためです。

こうして、なんの処理をしているのか一見してもわからないメソッドが誕生してしまうのです。

scope でなくクラスメソッドを使うのであれば、3-1. や 3-2. の問題を回避できる可能性が高まります。

# もっとも記事を書いている User を返す。Article が1件もないときのみ nil を返す
# @return [User]
def self.rank1_writer
  writer_ids = Article.pluck(:user_id)
  most_wrote_writer_id = writer_ids.group_by(&:to_i).values.max_by(&:size)&.first
  User.find_by(id: most_wrote_writer_id)
end

# super_admin もしくは admin の権限を持つ User の一覧を返す
# @return [Array<User>]
def self.admin
  undeleted_users = User.where(deleted_at: nil)

  undeleted_users.select do |user|
    user_role_keys = user.roles.map(&:key)

    user_role_keys.select do |key|
      %w(super_admin admin).include?(key)
    end
  end
end

なんの処理をするメソッドか書いたおかげで、
メソッド名のひどさが際立ちましたが(笑)、
ともかく、変数名の自己ドキュメント化の要素も加わり、どういう処理でどんな値を返すのか、とてもわかりやすくなりました。

余談ですが、 self.admin メソッドは N+1 問題を発生させているヤバいコードです。
undeleted_users の数だけSQLクエリが発行されます。

3-3. テストコードが書かれないケースが多い

scope がメソッドであると意識されないゆえにテストコードが書かれないケースが多いです。

冒頭で紹介した

scope :draft, -> { where(published_at: nil) }

であれば、そのまますぎるのでテストコードを書く必要もないわけですが、

scope :rank1_writer, -> { find_by(id: Article.pluck(:user_id).group_by(&:to_i).values.max_by(&:size).first) }
scope :admin, -> { where(deleted_at: nil).select{ |user| user.roles.map(&:key).select{ |key| %w(super_admin admin).include?(key) } } }

などの明らかに複雑な処理であればテストコードは書かれるべきです。
例えば rank1_writer メソッドは Article が1件もない場合に undefined method 'first' for nil:NilClass でエラーを吐くバグがありますしね。

4. scope を使うのであれば

チーム開発において scope を使うのであれば、
以下のルールを徹底すると多少マシになるかと思います。

4-1. ActiveRecord::Relation を返すように義務付ける

個人的には scope って ActiveRecord::Relation を返す目的で使うものだと思っていたのですが、
以下のように Array など別のクラスを返す scope がよくあります。

scope :latest_users, -> { order(created_at: :desc).first(5) }

これだと scope メソッド同士での連結ができませんし、
一度 Array になってしまうと使い道にも制限が出てしまうので、あまり良くないと思います。

4-2. コメントを書く・テストを書く

上の 3-2. および 3-3. で指摘したことの逆ですが、
scope であってもコメントを書くこと、テストを書くことをルール化しておけば、
コードの可用性は多少マシになると思います。

4-3. メソッドチェーンが3つ以上になるならクラスメソッドにする

Webアプリケーションの運用を続けていると、
既存メソッドの条件が変わっていくことは往々にしてあります。

例えば、活動しているライター一覧を表示するページに使う scope が以下のようであったとして、

scope :active_writers, -> { where(deleted_at: nil, type: User.types[:writer]).order(updated_at: :desc) }

「最近記事を書いているライターに限定してほしい」という声があったときに以下のように変更するとしましょう。

scope :active_writers, -> { left_joins(:articles).where(deleted_at: nil, type: User.types[:writer]).where("articles.created_at > ?", 1.month.ago).order(updated_at: :desc) }

このときに、「これが1行で書かれているのは処理の内容としても、見やすさとしても、わかりにくいのでは？」
と考えて、scope でなくクラスメソッドに変更するのは大いにアリだと思います。

5. でも scope を使う必要ってある？

5-1. ActiveRecord::Relation と Array の違いがわからない問題

4. のように対応案を書きましたが、実は 4-1. の時点で難しいです。
ちゃんと Rails がわかる人がレビュアーにいるうちはいいのですが、

Article.order(:created_at).last(3)

と

Article.order(created_at: :desc).limit(3)

の返すクラスが違うと知らない人だけになってくると、4-1. のルールすら守られなくなってきます。

「いやいや、 .class.name でつなげば何のクラスが返ってくるかわかるでしょ？」と思うかもしれませんが、
それはプログラミングに慣れている人の考えで、始めたての人はメソッドの返り値のクラスがなにかを意識していません。

5-2. DRY にしないほうが可読性いい説

scope は短いメソッドチェーンのものに限って使うのなら良いかもしれません。しかし、

scope :draft, -> { where(published_at: nil) }
scope :active_writers, -> { where(deleted_at: nil, type: User.types[:writer]).order(updated_at: :desc) }

のような単純なものにしか使わないのであれば、そもそも scope を使わなくていいかもしれません。

controller で使われているときに「このメソッドは実際どんな定義なんだろう？」と model 側を見に行く必要が発生しませんし、
どんなSQLクエリが発行されるのか、メソッドチェーンを見るだけで想像しやすい利点があります。

処理の同じところがあればまとめたくなるのは、プログラマーとして健全な考え方ですが、
Query Builder に関しては、ベタ書きする方が
結果的に可読性が高くバグを起こしにくくなることも多いのではと考えています。
（これは賛否両論あるでしょう）

# scope 使用
@active_writers = User.active_writers.page(params[:page]).per(20)

# scope を使わない場合
@active_writers =
  User
    .where(deleted_at: nil, type: User.types[:writer])
    .order(updated_at: :desc)
    .page(params[:page]).per(20)

6. 結論

というわけで個人的には、
「たとえ、既存コードのコピペでコードのほとんどを書くエンジニアが引き継ぐことになっても、ある程度回るようにしたい」
という前提に立つのであれば、scope に関しては

• scope を使わずクラスメソッドを使うこと
• メソッドには必ず yard に従ったコメントを付けること
• 各メソッドへのテストを書くこと
• default_scope は絶対に使わない（この記事で触れませんでしたが、バグの温床になりやすい）

あたりをルールとしてコードを書くかなあ、と思います。

もちろん、コードレビューの環境がしっかり整っているならこういったルールがなくとも品質が担保されると思うのですが、
プロダクトが長く続くものという前提に立ったとき、
「たとえ会社のエンジニア採用能力が落ちて、今いる人が全員いなくなった後にコピペエンジニアしか残らなかった」という場合を考慮すると、
コードが無法地帯にならないためにはこういったルールを先んじて制定しておくと安心度が高まるかなと考えています。