実はRailsでの開発を仕事とするようになって、そこそこ経ちました。
初めは、すげー、これ便利！ と感動することしきりだったわけですけれど、いざ実運用に乗せてみると苦戦することもたくさん。
今回は、実装するときは、ちょっと立ち止まって考えないと、はまってしまいそうな（ていうか、よくはまってた）事例をまとめてみようかと思います。
NoMethodErrorの罠
まず一番よく出るのが、NoMethodError。
たとえば、モデルのリレーションに
-
belongs_to :user
と指定したとしましょう。
で、ビュー側では以下のように書いていたとします。
-
entry.user.name
もしテーブルのuser_idがnilのデータを取得してきたら、NoMethodErrorが出てしまいますね。
ちょっと気を抜くと、このエラーが頻発します。
-
if entry.user.blank? ? entry.user.name : "名無しさん"
たとえば、必ずこんな感じにでも書くようにする、とか決まっていればいいのかもしれませんが、実はnilの入る可能性のある項目なのに、ここは必ず入力されているもの、とか思い込むと意識から飛んだり、そもそもその可能性を見落としたりすることがよくあります。
フィルターの罠
2つめは、フィルター絡みの事例。
Railsでは、コントローラの処理の前後にフィルターを挟むことができます。
そうすることで、共通の処理の手間を省こうという狙いですね。
でも、使い方を間違えると、かえって痛い思いをしたりします。
-
class HogeController ＜ ApplicationController
-
before_filter :get_mobile_info
-
before_filter :login_required
-
before_filter :profile_required
-
-
def index
-
#（中略）
-
end
-
-
before_filter :create_access_log
-
-
def search
-
#（中略）
-
end
-
-
after_filter :update_users_logined_at
-
end
たとえば、これは極端な例かもしれないけれど、誰かとソースが競合するのが嫌だからって書く場所を変えたりすると、こんなコードになったりします。
こんな風に、フィルターの定義を一番上と一番下にでも書かれたら、それだけで追いかけるのが難しくなります。
まして、真ん中の辺りにでも書かれようものなら、8割がた見落とせます。
さらに、コントローラの継承までやっていて、そこにも同じようにフィルターが挟まっていたら、メソッドを実行する前に、何がどれだけ動くのか把握できたものじゃありません。間接的なスパゲッティコード（しかも読みにくい。。）の誕生です。
簡単に書けてしまうから、と、ついつい使ってしまうけど、バグが起きたときの追跡の手間とかを考えると、ひとつに集約できたらいいのになー、と思わずにはいられません。
-
before_filter :login_required, :available => [{:controller => :home, :except =>"login"},
-
{:controller => :diary, :except => "show"}]
たとえばこんな感じで、どこかで一括指定でもできたらいいのに、と最近は思います。（何かしら、プラグインとかでありそうですが。。。）
トランザクションの罠
3つ目は、トランザクション。
Railsの場合、トランザクションは、特に何も指定しなければ、saveやらdeleteやらのタイミングで一個ずつ走ります。
そのため、場合によっては、いわゆる、プログラミング的トランザクションという奴を意識しないと、後々潜在的なバグに苦しめられることになります。
たとえば、自分の手持ちのポイントを払ってアイテムを買うような処理を考えてみます。
同じアイテムは一個しか買えないという仕様だったとしましょう。
-
class ItemController ＜ ApplicationController
-
def buy
-
item = Item.find_by_id(params[:id])
-
unless item
-
raise NotFoundException
-
end
-
if UsersItem.has_been_bought?(item, @login_user)
-
flash[:error] = "すでに購入済みですよ。"
-
redirect_to :action => "check"
-
return
-
end
-
users_item = UsersItem.new(:item_id => item.id, :user_id => @login_user.id)
-
users_item.save
-
buyer = User.find_by_id(@login_user.id)
-
buyer.point = buyer.point - item.point
-
buyer.save
-
rescue NotFoundException => e
-
flash[:error] = "そんなアイテムないですよ。"
-
redirect_to :action => "check"
-
return
-
end
-
end
-
class UsersItem ＜ ActiveRecord::Base
-
def self.has_been_bought?(item, user)
-
find_by_user_id_and_item_id(user.id, item.id)
-
end
-
end
みたいな感じかと思われます（適当に書いたから間違ってる予感も。。）。
こう書いた場合、UsersItemsテーブルへのsave処理が何かの理由で待ち状態になってしまっているときに、もし再度同じメソッドが実行されたら、あっさり直前のチェックロジックを通り抜けて、アイテムがめでたく2つ買えてしまうことになります。
もちろん、前後をActiveRecord::Base.transaction do～endで囲って、さらにsave後にもチェックロジックを挟んで、いざ引っかかったときはraiseしたものをrescueすれば、おそらく改善されるのでしょう（長い）。ソースで書けばこんな感じでしょうか。
-
class ItemController ＜ ApplicationController
-
def buy
-
item = Item.find_by_id(params[:id])
-
unless item
-
raise NotFoundException
-
end
-
if UsersItem.has_been_bought?(item, @login_user)
-
flash[:error] = "すでに購入済みですよ。"
-
redirect_to :action => "check"
-
return
-
end
-
ActiveRecord::Base.transaction do
-
users_item = UsersItem.new(:item_id => item.id, :user_id => @login_user.id)
-
if users_item.save
-
if UsersItem.count_by_item_id_and_user_id(item.id, @login_user.id) == 1
-
buyer = User.find_by_id(@login_user.id)
-
buyer.point = buyer.point - item.point
-
buyer.save
-
else
-
raise DuplicatePurchaseException
-
end
-
else
-
redirect_to :action => "check"
-
return
-
end
-
end
-
rescue NotFoundException => e
-
flash[:error] = "そんなアイテムないですよ。"
-
redirect_to :action => "check"
-
return
-
rescue DuplicatePurchaseException => e
-
flash[:error] = "すでに購入済ですよ。"
-
redirect_to :action => "check"
-
return
-
end
-
end
常にこれをやるのもあんまり賢くない感じがします。。
でも、厳密にやらなければいけないとき（たとえば、お金が絡むときとか）は、これくらい慎重でありたいものです。
きちんと動くようにするために
実はこういうのがまずい、こうしたほうがいい、というやり方は、運用を始めてみてようやくわかるのかもしれません。
そういうノウハウって、数をこなして、事例を経験していかないと蓄積されないんだろうなぁ。
0 Responses to “Railsでの実装で気をつけたい3つの罠”