Help us understand the problem. What is going on with this article?

Railsアンチパターン<モデル編>②Fat model

More than 5 years have passed since last update.

アプリケーションはなるべく単純な方が良い。modelの中にある複雑さを新しいどこか、例えばmoduleとかクラスとかに移動することによって単純にしていくというアプローチを考えてみよう。

以下のようなオンラインショッピングアプリケーションのためのモデルを考えてみる。クラスメソッドとして、状態によってorderを検索するメソッド、手段をしてして全検索するメソッド、結果をxml,json,pdfなどのフォーマットにexportするメソッドを持っている。

# app/models/order.rb
class Order < ActiveRecord::Base

  def self.find_purchased
    # ...
  end

  def self.find_waiting_for_review
    # ...
  end

  def self.find_waiting_for_sign_off
    # ...
  end

  def self.find_waiting_for_sign_off
    # ...
  end

  def self.advanced_search(fields, options = {})
    # ...
  end

  def self.simple_search(terms)
    # ...
  end

  def to_xml 
    # ...
  end

  def to_json 
    # ...
  end

  def to_csv 
    # ...
  end

  def to_pdf 
    # ...
  end 
end

さて、もうこの章で言わんとする問題がわかったと思う。この調子で開発をすすめていくと、あからさまにだるいメソッドでモデルがどんどん太っていく未来が見える。じゃあこれらのメソッドはどこにおけばいいんだ?以下に解決法をしめしていく。そのうち、実は裏側のドメインに問題があったりすることやなぜメソッドが違うクラスに移動できるのかについても触れよう。

ソリューション:新しいクラスに責任を委譲する

# app/models/order.rb
class Order < ActiveRecord::Base
#...
  def to_xml
    #... 
  end

  def to_json
    #..
    #(以下略)
end

実際こういう部分は明らかに本質でないので別モジュールに分けられる。Orderクラスはその名の通り、orderに関わることに対してのみ責任を持つべきだ。単一責任の法則。"クラスを変更する理由はたった一つでなければならない"。

別クラスに分けることによってその法則は守られる。

# app/models/order.rb
class Order < ActiveRecord::Base
  def converter 
    OrderConverter.new(self)
  end 
end

# app/models/order_converter.rb 
class OrderConverter
  attr_reader :order 

  def initialize(order)
    @order = order 
  end

  def to_xml 
    # ...
    #(以下略)
end

こうすることによって変換メソッドたちをそれ専用のクラスに追い出すことができた。object指向でいうところの「コンポジション」だ(コンポジションについて:参考)。
デメテルの法則のためにdelegateをするのを忘れないこと。

# app/models/order.rb
class Order < ActiveRecord::Base
  delegate :to_xml, :to_json, :to_csv, :to_pdf, :to => :converter 
  def converter
    OrderConverter.new(self) 
  end
end

次に銀行口座を表す以下のようなクラスを考えてみる。

# app/models/bank_account.rb
class BankAccount < ActiveRecord::Base
  validates :balance_in_cents, :presence => true validates :currency, :presence => true

  def balance_in_other_currency(currency) 
    # currency exchange logic...
  end

  def balance 
    balance_in_cents / 100
  end

  def balance_equal?(other_bank_account) 
    balance_in_cents ==
      other_bank_account.balance_in_other_currency(currency) 
  end
end

基本的な口座に対してのアクション(預金、引き出し、振込などなど)に加えて、金額をドルで返すメソッドや、残高を比較するメソッドまで備えてる。これはちょっと色々やりすぎだ。「口座」に対して行われるべきことと、預金という「お金」に対して行われるべきことがごちゃまぜになってしまっている。

「預金」に対して行っていることを別クラスに切り出そう。Railsにはこういうことを便利にするcomposed_ofメソッドというのがある。参照名、クラス名、そしてマッピング(リンクの例を読めばわかると思うが、複数のdelegateの対応表のようなものだ)を指定する。
これを使うと、

# app/models/bank_account.rb
class BankAccount < ActiveRecord::Base
  validates :balance_in_cents, :presence => true validates :currency, :presence => true
  composed_of :balance,
              :class_name => "Money",
              :mapping => [%w(balance_in_cents amount_in_cents), %w(currency currency)]
end

# app/models/money.rb 
class Money
  include Comparable
  attr_accessor :amount_in_cents, :currency

  def initialize(amount_in_cents, currency) 
    self.amount_in_cents = amount_in_cents self.currency = currency
  end

  def in_currency(other_currency) 
    # currency exchange logic...
  end

  def amount 
    amount_in_cents / 100
  end

  def <=>(other_money) 
    amount_in_cents <=>
      other_money.in_currency(currency).amount_in_cents 
  end
end

これで預金の「お金」としての操作、責任を委譲できた。ちなみにこれの一つ目とやってることほとんど同じ。詳しく解説されているので読むと良い。composed_ofについてのAPIドキュメントも読んだ方がいいかもしれない。

結論:そのメソッドは本当にそのクラスにあるべきか考えよう。クラスを分割してdelegateやcomposed_ofを使って適切なクラスに責任を委譲しよう。

ソリューション:モジュールを作る

モジュールという別の視点からModelをスリムにできないか考えてみる。
先のOrderクラスの話だと、大きくメソッドを三つにわけることができる。状態を指定してorderを探索するようなメソッド、すべてのorderを検索するようなメソッド、そしてexport関係のメソッド。これらをそれぞれにmoduleに分割してみる。

# app/models/order.rb
class Order < ActiveRecord::Base
  extend OrderStateFinders 
  extend OrderSearchers 
  include OrderExporters
end

# lib/order_state_finders.rb 
module OrderStateFinders
  def find_purchased 
    # ...
  end

  def find_waiting_for_review 
     # ...
  end

  def find_waiting_for_sign_off 
     # ...
  end

  def find_waiting_for_sign_off 
    # ...
  end 
end

# lib/order_searchers.rb 
module OrderSearchers
  def advanced_search(fields, options = {}) 
    # ...
  end

  def simple_search(terms)
    # ...
  end 
end

# lib/order_exporters.rb 
module OrderExporters
  def to_xml 
    # ...
  end
  #(以下略)
end

これも、複雑さを排除する一つの方法だ。ただし、先ほどあげたエントリでは安直にconcerns/以下にモジュールを切り出すことについては本質的な解決ではないと否定的だ。採用についてはよく考えるべきだろう。

結論:クラスでなく、モジュールに分割する手もある。ただし、これについては賛否両論だ。

ソリューション:トランザクションブロックを小さくする。

以下のようなaccountモデルをcreateするメソッドについて考えてみる。

class Account < ActiveRecord::Base
  def create_account!(account_params, user_params)
    transaction do
      account = Account.create!(account_params) first_user = User.new(user_params)
      first_user.admin = true
      first_user.save!
      self.users << first_user
      account.save! Mailer.deliver_confirmation(first_user) return account
    end
  end
end

RailsにはCallbackという便利な仕組みがあり、save,save!,destroyについてはコールバックもtransaction内で行われることになっている。それを利用しよう。

ネストした関連についてはaccepted_nested_attributes_forを使うこと(参考)。
また、何かしらの属性についてのフラグなんかをいちいち書いているのなら、Controllerのコールバックを使おう。

結論:callbackを最大限利用しよう。

汎用的な処理をコールバックに登録する際はこちらのエントリで触れられているようにそれ用のクラスを定義してしまうと、より責任の所在が分離できていいかも。

lastcat_
Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away