Railsを使っている際にはRails Best Practicesの内容を確認したりしているのですが、今までなんとなく「あー、この方がいいんだー」って眺めていただけなのですが、数が増えてきたので、いい加減ちゃんと読もうと思いました。
結構面白いものもあり、「へー」と感じたものを中心にまとめてみました。
更新履歴
2014/12/15 追加
Use Query Attribute
ActiveRecord の attribute に値が入っているかチェックする際に nil?, blank?, present? とかは使わなくていいって意味です。
代わりに 'attribute名' + '?' を使いましょうって話です。
悪い例
<% unless @member.name.present? %>
名無しの権兵衛
<% end %>
良い例
<% unless @member.name? %>
名無しの権兵衛
<% end %>
Law of Demeter
Law of Demeter とは日本語では「デメテルの法則」と呼ばれ、簡単にまとめると、a.b.c() という形でオブジェクトの関数などを介して、もう1つ先の関数などを呼び出すのは 良くない というものです。
悪い例
member.last_name.kana
良い例
class member
:
def last_name_kana
last_name.kana
end
:
end
member.last_name_kana
上の悪い例のようになんとか . なんとか ． 関数 とつなげていくのではなく、関数にしてしまい関数の中でその先を呼び出す形にするべきである、ということです。
こうすることにより、コンポーネント（上の場合はクラス）間の依存関係が少なくなり、いざというときの修正や変更が楽になる、ということです。
Replace Instance Variables with Local Variables
Partial View でも、インスタンス変数（@variable）を使えますが、partial を含んでいる View との依存関係が強まり、他のところで使いまわしにくくなります。
インスタンス変数ではなくローカル変数として、render で呼び出す際に明示的に渡した方がコードの見渡しが良くなります。
悪い例
コントローラ
def show
@members = Member.all <%# インスタンス変数に値を設定 %>
end
アクションのビュー（例：show.html.erb）
<%= render partial: 'member_names' %> <%# ここで暗黙に @members を渡しているのと同じ %>
パーシャル（例：_member_names.html.erb）
<%# @members に依存しているけど、このファイルを開くまでは分からない %>
<%= @members.map { |m| m.name } %>
良い例
アクションのビュー（例：show.html.erb）
<%= render patial: 'member_names', locals: { members: @members } %> <%# 明示的にローカル変数として渡している %>
注：上のコードは実は冗長です。下の 'Simplify Render in Views' 参照のこと
パーシャル（例：_member_names.html.erb）
<%# ローカル変数を使う %>
<%= members.map { |m| m.name } %>
Simplify Render in Views
render って実は結構省略して書けますって話です。
上の例を見てみましょう。
Before
<%= render partial: 'member_names', locals: { members: @members } %>
After
<%= render 'member_names', members: @members %>
partial や locals を省いても大丈夫です。だいぶスッキリしましたね。
Move Model Logic into Model
MVC (Model View Controller) ではコントローラはなるべく仕事をしない、というのが基本発想です。
なので、
class MembersController < ApplicationController
:
def update_phone_number
@member = Member.find(params[:id])
phone_number = phone_number_params[:phone_number]
@member.is_temporary = true
@member.phone_number = remove_non_digit(phone_number)
@member.save
end
:
end
と書いてしまうと、
- コントローラにロジックが多くなってしまう。
-
MembersControllerとMemberとの依存関係が強くなってしまう - 流用性が低くなってしまう（ロジックを他で使いにくくなってしまう）
といった弊害がうまれてしまいます。
コントローラではなく、モデルに書くようにしましょう。
class Member < ActiveRecord::Base
:
# ロジックはモデルの中に書く
def update_phone_number(phone_number)
self.is_temporary = true
self.phone_number = remove_non_digit(phone_number)
save
end
:
end
class MembersController < ApplicationController
:
def update_phone_number
@member = Member.find(params[:id])
phone_number = phone_number_params[:phone_number]
@member.update_phone_number(phone_number)
end
:
end
コントローラがすっきりしましたね１
Remove Empty Helpers
Rails 便利なんですが、controller とか scaffold を作ると自動的にいらない helper とかも作られちゃうんですよね。
あまり気持ちいいものでもないので、消してちゃいましょう。
Remove Trailing Whitespace
コードの行末に空白が残ってしまうと消したときとかに Diff に表示が出てしまって邪魔なので、コミットする前に消すようにしましょう！
と言っても気にしていないと簡単に入ってしまうので、エディタ設定で目立つようにするとか、勝手に消してくれるようにしましょう！
ちなみに Vim ユーザな僕は vim-trailing-whitespace を入れて、赤く表示されるようにしています。
さらに、:FixWhitespace と打つと一発で消してくれます。Vim 使いの方はどうぞ〜。
他にも commit hook を使う方法 っていうのもあります。こちらも設定しておくのがいいかもしれませんね。
Overuse route customizations
Rails の routes って気を付けてないと、美しないコードがどんどん追加されていきます。
最初は
「とりあえず1つしか Action ないし、get 1行でいいや」
get 'posts/get_post'
が
「あー、前の変えると他の場所も変えないと行けないから面倒だな―。post 1行追加でいいや。」
post 'posts/update_post'
get 'posts/get_post'
を繰り返し、やたらカスタムな（RESTFUL でない）Routes ができ上がってしまいます。
そうなる前になるべく早めに
resources :posts
みたいに Rails の標準にのっとったものを使うと、RESTFUL で綺麗なインターフェースが作れます。
Always add DB index
まあ、ちゃんと Index を作ろうねって話です。
特に Rails だと簡単に Migration で外部キーとかを作れてしまい、意外と忘れがちです。
SORT や GROUP BY も Ruby のコードで書けてしまうので、同様に意識しとかないと、やたら遅いクエリーができあがってしまいます。
class CreateUsers < ActiveRecord::Migration
def change
create_table "computer" do |t|
t.string :name
t.integer :os_id
t.timestamps
end
add_index :users, :os_id # <= これですね
end
end