無料で読めるRails antipatternから一つずつエッセンスをメモがてら。割と有用そうな本なのに訳されてないのは残念ですな。あくまでざっくりメモなのでご容赦ください。
ソリューション: デメテルの法則を参考に、delegateの使用を検討する
以下のようなモデル、関連があるとする。
class Address < ActiveRecord::Base
belongs_to :customer
end
class Customer < ActiveRecord::Base
has_one :address
has_many :invoices
end
class Invoice < ActiveRecord::Base
belongs_to :customer
end
それについて以下のようなviewを使ったとする
<%= @invoice.customer_name %>
<%= @invoice.customer.address.street %>
<%= @invoice.custome.address.city %>,
<%= @invoice.customer.address.state %>
<%= @invoice.customer.address.zip_code %>
これはカプセル化の概念に反している。もしcustomerが請求先住所と運送先住所を別々にもつようになったら、このようにオブジェクトをまたいで直接属性にアクセスしている箇所を全て修正しないといけない。
デメテルの法則というのがある。一言で言うと、.は一つだけにしておけ、という法則。たとえば@invoice.customer.address.streetなんてのは.が4つもあるのでこの観点から言うとよろしくない。まあもちろんこれは極論ではあるけど、従うべきガイドラインにはなると。
この法則を適用すると、
class Address < ActiveRecord::Base
belongs_to :customer
end
class Customer < ActiveRecord::Base
has_one :address
has_many :invoices
def street
address.street
end
def city
address.city
end
def state
address.state
end
def zip_code
address.zip_code
end
end
class Invoice < ActiveRecord::Base
belongs_to :customer
def customer_name
customer.name
end
def customer_street
customer.street
end
def customer_city
customer.city
end
def customer_state
customer.state
end
def customer_zip_code
customer.zip_code
end
end
するとviewのコードはこう変わります。
<%= @invoice.customer_name %>
<%= @invoice.customer_street %>
<%= @invoice.customer_city %>,
<%= @invoice.customer_state %>
<%= @invoice.customer_zip_code %>
.が一つになった。が、ラッパーメソッドでクラスが埋め尽くされてちょっとだるくなったり、invoiceクラスのインタフェースが汚くなる(wikipediaにも同様の欠点が書かれている)。が、Railsにはdelgateという便利な機能があるのでそれを使えば、
class Address < ActiveRecord::Base
belongs_to :customer
end
class Customer < ActiveRecord::Base
has_one :address
has_many :invoices
delegate :street, :city, :state, :zip_code, :to => :address
end
class Invoice < ActiveRecord::Base
belongs_to :customer
delegate :name,
:street,
:city,
:state,
:zip_code,
:to => :customer,
:prefix => true
end
とシンプルに書ける。
##結論:delegate使おう
ソリューション:生のfind()呼び出しを避け、モデルにfinderメソッドを実装する
とにかくドメインモデルをcontrller,viewから追い出す
そこらかしこでmodelのfinderをよんでしまうと保守性が下がる。
例えば、すべてのUserをlastnameで並び替えて表示したいとする。viewでそのままfindをよんでしまうと、
<html>
<body>
<ul>
<% User.find(:order => "last_name").each do |user| -%>
<li><%= user.last_name %> <%= user.first_name %></li>
<% end %>
</ul>
</body>
</html>
確かにこういうのも可能だが、MVCの原則を守るなら、以下のようにすべきだ。
class UsersController < ApplicationController
def index
@users = User.order("last_name")
end
end
<html>
<body>
<ul>
<% @users.each do |user| -%>
<li><%= user.last_name %> <%= user.first_name %></li>
<% end %>
</ul>
</body>
</html>
さらに特定の検索条件のところを、scopeを使ってモデルに寄せる。
class User < ActiveRecord::Base
scope :ordered, order("last_name")
end
これで、ロジックがモデルに収まった。
##結論:view、controllerにfinderロジックを書くな。モデルのscopeとか使おう。
ソリューション:そのモデルのfinderは「自身」の中に置いておくこと
アプリがもっと大きくなって、以下のようなちょっと複雑なfinderをコントローラーに書くことになった。
class UsersController < ApplicationController
def index
@user = User.find(params[:id])
@memberships =
@user.memberships.where(:active => true).
limit(5).
order("last_active_on DESC")
end
end
前述のソリューションを適用して、モデルのfinderメソッドに責任を委譲する。
class UsersController < ApplicationController
def index
@user = User.find(params[:id])
@recent_active_memberships = @user.find_recent_active_memberships
end
end
class User < ActiveRecord::Base
has_many :memberships
def find_recent_active_memberships
memberships.where(:active => true).
limit(5).
order("last_active_on DESC")
end
end
コントローラーは薄くなったが、もうちょっと改善できる。問題は、User.find_recent_active_membershipsがMembershipモデルの実装を知りすぎていることだ。責任を適切なモデルに委譲することで、少し改善できる。
class User < ActiveRecord::Base
has_many :memberships
def find_recent_active_memberships
memberships.find_recently_active
end
end
class Membership < ActiveRecord::Base
belongs_to :user
##ここ
def self.find_recently_active
where(:active => true).limit(5).order("last_active_on DESC")
end
end
さらにscopeを使うことによって
class User < ActiveRecord::Base
has_many :memberships
def find_recent_active_memberships
memberships.only_active.order_by_activity.limit(5)
end
end
class Membership < ActiveRecord::Base
belongs_to :user
scope :only_active, where(:active => true)
scope :order_by_activity, order('last_active_on DESC')
end
これにより、各々の操作の責任が適切なモデルに分散された。素晴らしい。ただし、見ればわかるように、
memberships.only_active.order_by_activity.limit(5)
少しデメテルの法則に反してしまっている。まあ関連についてさらにメソッドを呼び出す、という流れになるので多少は仕方ないのだけど。すべてのアンチパターン(の対策)は銀の弾丸というわけではないので、それぞれの功罪と自分のアプリケーションの未来を踏まえて適切に判断していくことが重要だ。
##結論:finderメソッドは適切なモデル(主に対象モデル)に責任を委譲しよう。