Rails アンチパターン - ファットコントローラー（Fat Controller）

引き続きRails AntiPatternsという本を読んでいます。

https://www.amazon.co.jp/dp/B004C04QE0/ref=dp-kindle-redirect?_encoding=UTF8&btkr=1

前回は2 Modelsの１つめ「Voyeuristic Models」についてまとめました。

waterlow2013.hatenablog.com

今回は4 Controllersの２つめ「Fat Controller」についてまとめていきます。 Solutionが２つあるのですが、今回は１つ目について書きます。

Fat Controller

コントローラーからビジネスロジックを削除し、それをモデルに適切に配置します。
コールバック、セッター、データベースのデフォルトなど、Active Recordによって提供される機能は、このタスクの重要なツールセットです。

Solution: Use Active Record Callbacks and Setters

サンプルコード

class ArticlesController < ApplicationController
  def create
    @article = Article.new(params[:article])
    @article.reporter_id = current_user.id
    begin
      Article.transaction do
        @version = @article.create_version!(params[:version], current_user)
      end
    rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordInvalid
      render :action => :new and return false end
    end
    redirect_to article_path(@article)
  end
end

コントローラで手続き型っぽく書いてしまっているというざっくりとした理由の他に、以下のような具体的理由があります。

• transactionをコントローラで呼んでいる
  • ステートメントが互いに依存しているとだめ（？）
  • saveメソッド等とコールバックは勝手にトランザクションで貼られる
• saveが使われていない
  • transactionを貼らないといけない？
• 例外を制御フローに使っている
  • 例外的状況ではない
  • GOTO文と可読性が変わらない

目標

class ArticlesController < ApplicationController
  def create
    @article = Article.new(article_params)
    if @article.save
      redirect_to article_path(@article)
    else
      render :action => :new
    end
  end
end

ActiveRecordのsaveメソッドの戻り値で判断します。scaffoldで作られたものと同じですね。
create_version!メソッドを消す必要があるので、見ていきましょう。

def create_version!(attributes, user)
  if self.versions.empty?
    return create_first_version!(attributes, user)
  end

  # mark old related links as not current
  if self.current_version.relateds.any?
    self.current_version.relateds.each do |rel|
      rel.update_attribute(:current, false)
    end
  end

  version = self.versions.build(attributes)
  version.article_id = self.id
  version.written_at = Time.now
  version.writer_id = user.id
  version.version = self.current_verison.version + 1
  self.save!
  self.update_attribute(:current_version_id, version.id)
  version
end

def create_first_version!(attributes, user)
  version = self.versions.build(attributes)
  version.written_at = Time.now
  version.writer_id = user.id
  version.state ||= "Raw"
  version.version = 1
  self.save!
  self.update_attribute(:current_version_id, version.id)
  version
end

必要に応じて他のメソッドversionモデルも見ていきます。

消せるものを消す。

• version.written_at = Time.nowはcreated_atとupdated_atで代用できる。
• version.state ||= "Raw"はDBのデフォルト値を使う（必要ならgemをつかってもいい）
• version.article_id = self.idはself.versions.buildで勝手に入る。

def create_version!(attributes, user)
  if self.versions.empty?
    return create_first_version!(attributes, user)
  end

  # mark old related links as not current
  if self.current_version.relateds.any?
    self.current_version.relateds.each do |rel|
      rel.update_attribute(:current, false)
    end
  end

  version = self.versions.build(attributes)
  version.writer_id = user.id
  version.version = self.current_verison.version + 1
  self.save!
  self.update_attribute(:current_version_id, version.id)
  version
end

def create_first_version!(attributes, user)
  version = self.versions.build(attributes)
  version.writer_id = user.id
  version.version = 1
  self.save!
  self.update_attribute(:current_version_id, version.id)
  version
end

コールバックをつかう

コールバックにするのは以下の３つ。

• version.versionの設定
• current_version.relatedsのupdate
• self.update_attribute(:current_version_id, version.id)

version.versionの設定

class Version < ApplicationRecord
  before_validation :set_version_number, on: :create
  validates :version, presence: true

  private

  def set_version_number
    self.version =
      (article.current_version ?
        article.current_version.version : 0) + 1
  end
end

self.versions.empty?はself.current_versionの存在確認でOK。create_first_version!メソッドが必要なくなるので。以下のようになる。

def create_version!(attributes, user)
  # mark old related links as not current
  if self.current_version && self.current_version.relateds.any?
    self.current_version.relateds.each do |rel|
      rel.update_attribute(:current, false)
    end
  end

  version = self.versions.build(attributes)
  version.writer_id = user.id
  self.save!
  self.update_attribute(:current_version_id, version.id)
  version
end

* current_version.relatedsのupdate

class Version < ApplicationRecord
  before_validation :set_version_number, on: :create
  before_create :mark_related_links_not_current, 
                          if: :current_version
  validates :version, presence: true

  private

  def current_version
    article.current_version
  end

  def set_version_number
    self.version = (current_version ? current_version.version : 0) + 1
  end

  def mark_related_links_not_current
    current_version.relateds.each do |rel|
      rel.update_attribute(:current, false)
    end
  end
end

途中細かなリファクタリングをはさんでいますが、最後の結果だけ書いています。

def create_version!(attributes, user)
  version = self.versions.build(attributes)
  version.writer_id = user.id
  self.save!
  self.update_attribute(:current_version_id, version.id)
  version
end

だいぶスッキリしてきました！

self.update_attribute(:current_version_id, version.id)

class Version < ApplicationRecord
  before_validation :set_version_number, on: :create
  before_create :mark_related_links_not_current, 
                          if: :current_version
  after_create :set_current_version_on_article
  validates :version, presence: true

  private

  def set_current_version_on_article
    article.update_attribute :current_version_id, self.id
  end

  def current_version
    article.current_version
  end

  def set_version_number
    self.version = (current_version ? current_version.version : 0) + 1
  end

  def mark_related_links_not_current
    current_version.relateds.each do |rel|
      rel.update_attribute(:current, false)
    end
  end
end

メインのデータ更新以外は全てコールバックに、という流れですね。 create_version!消すまではもう一息という感じです。

def create_version!(attributes, user)
  version = self.versions.build(attributes)
  version.writer_id = user.id
  self.save!
  version
end

create_version!を消す

ここでcreate_version!を消します。 create_version!のversionに関するコードはコントローラに移します。

class ArticlesController < ApplicationController
  def create
    @article = Article.new(params[:article])
    @article.reporter_id = current_user.id
    @version = self.versions.build(attributes)
    @version.writer_id = current_user.id

    if @article.save
      redirect_to article_path(@article)
    else
      render action: :new
    end
  end
end

これで目標とする形にかなり近づきました。

その先

この書籍ではnested_attributesを使う方法が書かれていました。最近のRailsバージョンだとaccept_nseted_attributes_forを使うのかと思いますが、実はこの機能、頃合いを見て消される方針のようです。

https://github.com/rails/rails/pull/26976#discussion_r87855694

代替としては、自分が知っているのはFormオブジェクト使うやり方があるのですが、だいぶ本に載っている内容からはそれるので今回はパスします。 Formオブジェクトの作り方はいろいろあると思いますが、パーフェクトRailsあたりに載っていた気がしますし、有料ですがgorailsにも。

gorails.com

まとめ

fat controller解消は何かパターンがあるわけではなく、リファクタリングとコールバックの積み重ね、最終的にどうしようもない所だけFormオブジェクトにするという泥臭い感じになるのだなと思いました。ただこうゆうことをサクッとできないとfat controllerと付き合うしか無いと思うので、この辺できるようにしておきたいです。
scaffoldに近づけるのが理想的という話はめちゃくちゃ同意できました！

[2017-05-07 20:50追記] ２つ目の解決策であるMove to a Presenterはactive_presenterというgemを使うのですが、今はメンテされていない様子です。内容としては上で言うFormオブジェクトっぽいものを宣言的にくためのgemのようでした。こちらに関しては深入りしないことにします。