16
17

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

Railsアンチパターン<コントローラー編>②Fat Controller

Posted at

※①は諸認証gemの比較がちょっと重そうなので後回し。

タイトルの通り、Controllerにドメインロジックが書かれまくっている状態。Railsの標語の一つとして"Skinny Controller, Fat model"というのがあるが(Fat modelはそれはそれで問題で、取り上げる順番としては逆の方がいいのではと思ったけど、おそらく単にMVCの順番に章立てしたかったんだと思う)、それに失敗した実装のことを指すのが"Fat Controller"という言葉。その二通りの解決法を示す。ただし、後者についてはちょっと意見がある(後述)

ソリューション:ActiveRecordの機能をちゃんと利用する(ために実装をモデルに寄せる)

Railsの機能をちゃんと把握しよう編。以下のコードをリファクタリングする。

コントローラー

.rb
#@controller
class ArticlesController < ApplicationController 
  def create
    @article = Article.new(params[:article]) 
    @article.reporter_id = current_user.id
    begin
      Article.transaction do # => CRUD動作のcallbackにすることでtransactionの中に含める
        @version = @article.create_version!(params[:version],current_user)
      end
    rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordInvalid # => この程度は例外を使うべきではない
      render :action => :new and return false 
    end
    redirect_to article_path(@article)
  end
#...

モデル

.rb
#@model

def create_version!(attributes, user) 
  if self.versions.empty?
    return create_first_version!(attributes, user) 
  end
  # mark old related links as not current # => conditional callbackを使え
  if self.current_version.relateds.any? # 関連はnilを返さないので無駄
    self.current_version.relateds.each { |rel| rel.update_attribute(:current, false) }
  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                          # デフォルトのcreated_atで良い
  version.writer_id = user.id                            #
  version.state ||= "Raw"                                # DBのデフォルト値機能を使う
  version.version = 1                                    #
  self.save!                                             #
  self.update_attribute(:current_version_id, version.id) # この辺りのlogicはコントローラーに移す
  version                                                #
end

主にコメントに書いているような点を改善し、以下のようなモデル、コントローラーになった。
コントローラー

.rb
class ArticlesController < ApplicationController 
  def create
    @article = Article.new(params[:article]) 
    @article.reporter = current_user 
    @article.new_version.writer = current_user
    if @article.save
      render :action => :index
    else
      redirect_to article_path(@article)
    end 
end

モデル

.rb
class Version < ActiveRecord::Base
  before_validation :set_version_number, :on => :create 
  before_create :mark_related_links_not_current, :if => :current_version 
  after_create :set_current_version_on_article

  private

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

##結論: Railsの機能を使おう。

ソリューション:Preseterに移動

複数のモデルを同時に取り扱うControllerについて考える。例えばUserとそれに紐づくAccountを同時に作成するようなアクションを考える。

.rb
def create
  @account = Account.new(params[:account]) 
  @user = User.new(params[:user]) 
  @user.account = @account
  if @account.save and @user.save
    flash[:notice] = 'Account was successfully created.' redirect_to(@account)
  else
    render :action => "new"
  end
end

さて上の実装には実はバグがある。account認証に成功してuser parameterが異常だとaccountはinvalidなuserと紐付けられたまま保存されてしまう。
これを解決するために、二つのtransactionを同じレイヤーする必要がある。すなわち、片方でも失敗したら全体がroll backされる。
そこで二つを一つのtransactionで包んで失敗したらActiveRecordの例外を投げることにするわけだけど、

.rb

def create
  @account = Account.new(params[:account]) 
  @user = User.new(params[:user]) 
  @user.account = @account
  ActiveRecord::Base.transaction do 
    @account.save!
    @user.save!
  end
  rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved 
    render :action => "new"
end

この時点(p148の下らへん)ですでにコードから漂う臭いがやばい。
そもそも例外は例外的なことが起こった時に使うのではなかったか。validation失敗程度で呼んでるのどうなんだ、コントローラーでtransaction読んでるのも厳しい。

その解決策としてPresenter Patternを使う。
presenerは複数のモデルを編成する。一つのActiveRecordモデルのように振る舞いながら、幾つかのモデルの振る舞いを統合する。ActivePresenterというgemで提供されている。
大体こんな感じで使える。

プレゼンター

.rb
# app/models/signup.rb
class Signup < ActivePresenter::Base
  before_save :assign_user_to_account 
  presents :user, :account
  private
  def assign_user_to_account user.account = account
  end
end

ビュー

.erb
# app/views/signups/new.html.erb
<h1>Signup!</h1>
<%= form_for(@signup) do |f| %>
...

コントローラー

.rb

# app/controllers/signup_controller.rb
class SignupsController < ApplicationController
  def create
    @signup = Signup.new(params[:signup])
  if @signup.save
    redirect_to root_url
  else
    render :action => "new"
  end
end

##結論:複数のモデルを同時に扱う際はPresenter層を検討しよう(?)

#所感
ところでこれ巷で言われているPresenter層と明らかにやっていることが違う。よく言われているのはモデル(オブジェクト)の状態などに応じてhelperメソッドを展開する、みたいな印象なんだけど(decorationとも呼ばれている)、これは複数のモデルオブジェクトをひとかたまりで扱う方法を提供しているようだ。
このgemも最終コミット2012年で完全に放置されてる感じっぽいし(一応2014年にRails4対応のPRがあるけど放置されている)、このアプローチに他の名前がついたのか(実際プレセンテーションという言葉の意味が明らかに変わっている)、それともこのアプローチ自体が認められなかったのか、どうなんだろう。事情お知りの方がいらっしゃったら、伺いたいです。

16
17
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
16
17

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?