Rails
Rails-best-practice

Rails Best Practices の警告をちゃんと考える

Railsを使っている際にはRails Best Practicesの内容を確認したりしているのですが、今までなんとなく「あー、この方がいいんだー」って眺めていただけなのですが、数が増えてきたので、いい加減ちゃんと読もうと思いました。

結構面白いものもあり、「へー」と感じたものを中心にまとめてみました。


更新履歴


2014/12/15 追加


Use Query Attribute

http://rails-bestpractices.com/posts/56-use-query-attribute

ActiveRecord の attribute に値が入っているかチェックする際に nil?, blank?, present? とかは使わなくていいって意味です。

代わりに 'attribute名' + '?' を使いましょうって話です。

悪い例

<% unless @member.name.present? %>

名無しの権兵衛
<% end %>

良い例

<% unless @member.name? %>

名無しの権兵衛
<% end %>


Law of Demeter

http://rails-bestpractices.com/posts/15-the-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

http://rails-bestpractices.com/posts/27-replace-instance-variable-with-local-variable

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

http://rails-bestpractices.com/posts/61-simplify-render-in-views

render って実は結構省略して書けますって話です。

上の例を見てみましょう。


Before

<%= render partial: 'member_names', locals: { members: @members } %>


After

<%= render 'member_names', members: @members %>

partiallocals を省いても大丈夫です。だいぶスッキリしましたね。


Move Model Logic into Model

http://rails-bestpractices.com/posts/7-move-model-logic-into-the-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

と書いてしまうと、


  • コントローラにロジックが多くなってしまう。


  • MembersControllerMember との依存関係が強くなってしまう

  • 流用性が低くなってしまう(ロジックを他で使いにくくなってしまう)

といった弊害がうまれてしまいます。

コントローラではなく、モデルに書くようにしましょう。

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

コントローラがすっきりしましたね1


Remove Empty Helpers

http://rails-bestpractices.com/posts/72-remove-empty-helpers

Rails 便利なんですが、controller とか scaffold を作ると自動的にいらない helper とかも作られちゃうんですよね。

あまり気持ちいいものでもないので、消してちゃいましょう。


Remove Trailing Whitespace

http://rails-bestpractices.com/posts/60-remove-trailing-whitespace

コードの行末に空白が残ってしまうと消したときとかに Diff に表示が出てしまって邪魔なので、コミットする前に消すようにしましょう!

と言っても気にしていないと簡単に入ってしまうので、エディタ設定で目立つようにするとか、勝手に消してくれるようにしましょう!

ちなみに Vim ユーザな僕は vim-trailing-whitespace を入れて、赤く表示されるようにしています。

スクリーンショット 2014-12-03 18.12.17.png

さらに、:FixWhitespace と打つと一発で消してくれます。Vim 使いの方はどうぞ〜。

他にも commit hook を使う方法 っていうのもあります。こちらも設定しておくのがいいかもしれませんね。


Overuse route customizations

http://rails-bestpractices.com/posts/10-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

http://rails-bestpractices.com/posts/21-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