おもしろwebサービス開発日記

Ruby や Rails を中心に、web技術について書いています

我々はConcernsとどう向き合うか

この文章は先日開催された大阪Ruby会議02での登壇内容Concerns about Concernsをブログエントリにしたものです。書いている内容は登壇内容とだいたい同じですが完全一致ではなく、構成を変更したり喋っていない情報を足したりしてます*1。

大阪Ruby会議02に出席していない方でもスライドを読めば大体の内容を把握できると思いますが、これだと細かいニュアンスは伝えられない(し、この手の話はその細かいニュアンスが大事だったりする)のでちゃんとブログエントリにしておこうと思ったのでした。

意見がある人はこちらのスレに書いてもらえると嬉しいです(\( ⁰⊖⁰)/)

Concernsとはなにか

Concernsという概念は、Rails 4.0から導入されました。具体的にはrails newしたときに生成されるファイルたちの中に

  • app/models/concerns
  • app/controllers/concerns

という2つの空のディレクトリが追加され、かつConcernsを定義するのに便利なモジュールActiveSupport::Concernが追加されました。基本的にはこれだけです。これだけなのですが、僕たちにConcernsという概念を認識させるには十分でした。

使い方に関してはDHHのブログに書かれています*2。

Put chubby models on a diet with concerns – Signal v. Noise

このブログに端を発して、いろんなブログや書籍などでConcernsの説明が書かれましたが、基本的には「Concernsは関心事を分離するものである」というようなことが書いてあるはずです。みなさんもそのように認識しているのではないでしょうか。

例えばDHHのブログ中では次のようなサンプルコードが書かれています。

module Taggable
  extend ActiveSupport::Concern

  included do
    has_many :taggings, as: :taggable, dependent: :destroy
    has_many :tags, through: :taggings 
  end

  def tag_names
    tags.map(&:name)
  end
end

このようにモデル中のタグ機能を分離するのはわかりやすい例ですね。しかし「関心事を分離する」というのは結構難しい作業です。僕は仕事柄いろんな会社のいろんなRailsプロジェクトのコードを読むのですが「名前をつけるのが難しいが処理としては複数のクラスで使われているので適当な単位でモジュールとして切り出されているなにか」をよく見かけます*3。

DHHは上記のブログエントリ内で、Concernsを使うとモデルの本質的ではない箇所を「別クラスに切り出して単一責任原則」とかせずに切り出せて便利である。と言っているように見えます(下記の意訳なのですが間違ってたらごめんなさい)。

Concerns are also a helpful way of extracting a slice of model that doesn’t seem part of its essence (what is and isn’t in the essence of a model is a fuzzy line and a longer discussion) without going full-bore Single Responsibility Principle and running the risk of ballooning your object inventory.

僕個人としては、このConcernsの使い方はよくないと考えています。恐らくDHHの所属するBasecamp社のエンジニアはみんな腕利きで、Concernsを大量に使っていても破綻せずに開発できているのではないかと思いますが、そのような会社はごくごく少数です。普通の会社でDHHの書いていることを鵜呑みにしてConcernsを多用すると負債になってしまうのではないでしょうか。

Concernsアンチパターン

ではどのようにConcernsを扱うのが良いのかをアンチパターンを提示し、その改善案を出すという流れで書いていこうと思います。

コントローラのビジネスロジックをConcernsにする

次のようなコントローラがあったとします。

class PostsController < ApplicationController
  def show
    @post = Post.find(params[:id])
    @same_category_posts = same_category_posts(@post)
  end

  # 他のアクション...

  private

  def same_category_posts(post)
    category_ids = post.category_ids
    same_category_post_ids = PostCategory.where(category_id: category_ids)
                                         .pluck(:post_id)
    Post.where(id: same_category_post_ids - [post.id])
        .includes(:categories)
        .order(updated_at: :desc).limit(5)
  end  
end

ここでprivateメソッドとして書かれているsame_category_postsは、引数として受け取ったPostオブジェクトと同カテゴリのPostオブジェクトを最大5つ返すメソッドです。

仮に、このメソッドが他のコントローラでも定義されていたとします。そうするとDRYにしたくなりますよね。ではConcernsとしてモジュールに切り出してみましょう。

module PostFindable
  extend ActiveSupport::Concern
  
  def same_category_posts(post)
    category_ids = post.category_ids
    same_category_post_ids = PostCategory.where(category_id: category_ids)
                                         .pluck(:post_id)
    Post.where(id: same_category_post_ids - [post.id])
        .includes(:categories)
        .order(updated_at: :desc).limit(5)
  end
end

はい。そのままモジュールとして切り出しました。するともとのコントローラは次のようになります。

class PostsController < ApplicationController
  include PostFindable

  def show
    @post = Post.find(params[:id])
    @same_category_posts = same_category_posts(@post)
  end

  # 他のアクション...

メソッドが減って、なんとなくスッキリしたような気がしますね。しかしこれは良くないリファクタリング方法です。

そもそもsame_category_postsメソッドはビジネスロジックであり、コントローラに書くべきものではないからです。そこでsame_category_postsをPostモデルに書いてみます*4。

class Post < ApplicationRecord
  def same_category_posts
    same_category_post_ids = PostCategory.where(category_id: category_ids)
                                         .pluck(:post_id)
    Post.where(id: same_category_post_ids - [id])
        .includes(:categories)
        .order(updated_at: :desc).limit(5)
  end
end

するとコントローラは次のようになります。

class PostsController < ApplicationController
  def show
    @post = Post.find(params[:id])
    @same_category_posts = @post.same_category_posts
  end

  # 他のアクション...

先程のConcernsを使った方法と対して変わらないのでは？という人もいるかも知れませんが、これはサンプルコードが単純だからそう見えるだけです*5。仕事で開発するRailsアプリケーションはもっと複雑なビジネスロジックを多数含んでいます。そのビジネスロジックがコントローラ、モデルの両方に存在していると、特定の処理を追いかけるときにいろんな場所を見に行かなければならず、コードの見通しが悪くなります。コントローラのConcernsはコントローラのコンテキストに存在するので、仮にコントローラそのものからメソッドが見えなくなっても「ビジネスロジックをモデルに寄せる」という原則からは外れることになります。

rubocopのClassLength対策でConcernsにする

開発フローにrubocopを取り入れており、ClassLength設定が有効なプロジェクトがあったとします。仮にClassLengthが300だとすると、次のように何らかの修正の結果Postモデルが300行を超えたときにCIが失敗します。

class Post < ApplicationRecord
  # たくさんのロジックが300行ある
end

CIが失敗したときにPostモデルを修正していた人は、CIを通すため、なんとしてでもPostの行数を減らさなければなりません。そこで次のようにしてみます。

class Post < ApplicationRecord
  include Previewable
  include Reservable
  include Bookmarkable
end

ここで新しく定義したPreviewable, Resarvable, BookmarkableはもちろんConcernsとしてモジュールに切り出したものです。これでPostの行数は劇的に削減され、ClassLengthに引っかかることはなくなりました。めでたしめでたし…本当にそうでしょうか？

これらのConcernsはPostでのみ参照されるモジュールです。そしてPostの持っているメソッドは何一つとして減っていません。単にpost.rbから別のファイルに移動しただけで、実行時にはPostモデルのメソッドとしてこれまでと同様に振る舞います。

このようなケースでは、Concernsとしてではなく別クラスとして切り出し、Postモデルの責務を減らす必要があります。このアンチパターンは抽象的*6なので、別のクラスとして切り出す実例は次の「複雑なビジネスロジックをConcernsにする」を参照してください。

現実にはこのアンチパターンを擁護する人もいそうです。そのときに考えられる意見としては「コードの塊に名前がついて切り出されたぶん少し可読性が上がったのでは？」というのがあります。しかし、Active Recordは同じことを同一ファイル内で実現する方法を提供してくれているので「名前をつけて切り出す」のであればこちらの方法で良さそうです。

class Post < ApplicationRecord
  concerning :Previewable do
    # ...
  end

  concerning :Reservable do
    # ...
  end

  concerning :Bookmarkable do
    # ...
  end
end

参考: Module::Concerning

rubocopのClassLength警告は「該当クラスが責務を持ちすぎである」ということの指摘であるはずなので、実質的な責務を減らさないConcernsは意味がないですし、モデルに所属しているコードが追いづらくなるのでかえって可読性を下げる結果になるのではないでしょうか。

複雑なビジネスロジックをConcernsにする

次のモジュールは、ぱっと見ただけだとなにをするものなのか分かりづらいですね。引数として渡されたjsonの形式を見て、モデルを更新または削除するメソッドupdate_by_api!を提供するものです。

module UpdatableByApi
  extend ActiveSupport::Concern

  def update_by_api!(json)
    parsed_json = JSON.parse(json)

    if parsed_json['_destroy'].to_i == 1
      resouce.destroy!
    else
      raise '不正な値です' if parsed_json['reason'].blank?
      resource.attributes = parsed_json.except('_destroy').merge('updated_by' => 'api')
      resource.save!
    end
  end
end

(このくらいならすんなり読めるよ、という人もいるかも知れませんが)全体として何が行われているか、すぐにはわかりづらいコードであると感じます。リファクタリングしてみましょう。モジュールではなく、別クラスに切り出す(委譲)の方式で試してみます。

class UpdatingByApi
  def self.call(json)
    new(json).call
  end

  def initialize(json)
    @json = json
  end

  def call
    if destroy?
      resource.destroy!
    else
      raise '不正な値です' if invalid?
      resource.update!(new_attributes)
    end
  end

  private

  attr_reader :json

  def parsed_json
    @parsed_json ||= JSON.parse(json)
  end

  def destroy?
    parsed_json['_destroy'].to_i == 1
  end

  def invalid?
    parsed_json['reason'].blank?
  end

  def new_attributes
    parsed_json.except('_destroy').merge('updated_by' => 'api')
  end
end

行数は長くなりましたが、次のような効能が得られました。

  • それぞれの処理の詳細がプライベートメソッドに切り出されたことで詳細を把握しやすくなった
  • #callはプライベートメソッドの名前として説明されているメソッド名を読むことで、詳細を見ずに概要を把握できるようになった

今回わざわざモジュールからクラスに変更してリファクタリングしましたが、「プライベートメソッドとして切り出す」だけならモジュールでもできそうです。モジュールで同様にプライベートメソッドへの切り出しをやってみましょう。

module UpdatableByApi
  extend ActiveSupport::Concern

  def update_by_api!(json)
    if destroy?
      destroy!
    else
      raise '不正な値です' if invalid?
      update!(new_attributes)
    end
  end

  private

  def parsed_json
    @parsed_json ||= JSON.parse(json)
  end

  def destroy?
    parsed_json['_destroy'].to_i == 1
  end

  def invalid?
    parsed_json['reason'].blank?
  end

  def new_attributes
    parsed_json.except('_destroy').merge('updated_by' => 'api')
  end
end

同じようにできましたね。ではわざわざクラスにする必要はないのでしょうか？

そんなことはありません。この例だと、invalid?メソッドがActive Recordの提供するinvalid?メソッドをオーバーライドしてしまっているため、どこか別のコードが上手く動かなくなっている可能性が高いです。

ではinvalid?をリネームしたら良いでしょうか？しかし他のメソッド名はどうでしょう。Active Recordの提供するメソッド名とは重複していませんが、別のモジュールやモデルで普通に定義していそうな名前ですよね。また、@parsed_jsonインスタンス変数も同様です。モジュールでプライベートメソッドやインスタンス変数を定義する場合、すべて他のモジュールやinclude先のクラスの定義と重複しないように気をつけて実装する必要があります。

では、Concernsはどのように使うとよいのでしょうか。例えば次のようにupdate_from_api!というインタフェースだけを提供するモジュールとして実装するという方法があります。update_from_api!メソッドの実態は別クラスとして実装することで、includeで手軽にメソッドを増やせるのと、コンテキストを分けることで実装をリファクタリングしやすくなるメリットがあります。

module UpdatableByApi
  def update_form_api!(user, json)
    UpdatingByAPI.call(user: user, json: json)
  end
end

hookをConcernsにする

次のように、Active RecordのコールバックをConcernsにしているコードを時々見かけます。

module PublishedAtSettable
  extend ActiveSupport::Concern

  included do
    before_create :set_published_at
  end

  private

  def set_published_at
    self.published_at ||= Time.zone.now
  end
end

class Post < ApplicationRecord
  include PublishedAtSettable
end

しかし、コールバックを共通化して複数のクラスに適用したいのであれば、次のようにコールバックメソッド(この例だと before_create)にオブジェクトを渡すやり方でも実現できます。

class SetPublishedAt
  def before_create(publishable)
    publishable.published_at ||= Time.zone.now
  end
end

class Post < ApplicationRecord
  before_create SetPublishedAt.new
end

こちらのほうが、「Postモデルはbefore_createコールバックを定義している」ということがわかりやすくなり可読性が高いのではないでしょうか。

まとめ

今回の話で特に言いたかったことをまとめると次の3点です。

  • Concerns(module)はinclude先とコンテキストを共有するので、クラスから単純にConcernsに切り出しても責務が減るわけではない
  • 責務を減らしたいのであればクラスとして切り出したほうが良い
  • 安易にConcernsとして切り出すのではなく、まず別の方法も検討してみましょう

Concernsとして処理を切り出すと、目の前からコードがなくなるのでなんとなくリファクタリングできた気分になりますが、これは部屋にあるたくさんの荷物を部屋の隅に寄せて掃除をした気分になるのに似ていると感じます。これからConcernsを使うときには、これで本当に可読性が上がるのか一度考えてみてもらえると嬉しいです！

*1:例えばActiveSupport::Concernの説明は省略しています。ググればわかるので

*2:Railsガイドなど公式のドキュメントには特に言及がありません

*3:複数のクラスで使われていないけどなぜか切り出されているモジュールもたまに見かけます

*4:複数モデルが関わるメソッドなので別のクラスを作りそこにメソッドを定義すべき、という人もいるかも知れませんが、今回の主題から逸れるので一旦Postモデルで話をすすめます

*5:シンプルなコード例でもっといい感じに違いを表現できる方法が思いつかなかったのでした…

*6:実際に300行のPostモデルを用意するのは難しすぎ