railsの良い書き方を提案してくれる rails_best_practice

Railsにはrails_best_practiceというGemがあるのですが、
これは今時の書き方を提案してくれる便利なGemです。
URL: https://github.com/railsbp/rails_best_practices

このgithubページにいくつか実装されている内容が書かれているのですが、個人的に良さそうだなと思ったところだけピックアップして書きたいと思います。

the Law of Demeter(デメテルの法則）

これは、デメテルの法則に反するという警告を指し示しています。デメテルの法則とは、オブジェクトは直接関わっているオブジェクトの状態だけを把握しているというソフトウェア設計の概念です。Railsではオブジェクト指向の設計なので、特に設計の概念に適用されるでしょう。
悪い例が、

@vote.user.name

良い例が、

@vote.user_name

メソッドを二段階で呼び出すということをやっているというのは、デメテルの法則に反するということです。デメテルの法則は、オブジェクトは直接関わっているオブジェクトの状態だけを把握している概念です。なので、間接的にメソッドを呼び出すような処理は好ましくないということです。

Railsのモデルに書かれている処理にこれの警告が出ます。

class Vote < ActiveRecord::Base
  belongs_to :user
end

<%= @vote.user.name %>

ActiveSupportには、delegateというインスタンスメソッドが存在するので、そちらを使ってあげましょうとのこと。

class Vote < ActiveRecord::Base
  belongs_to :user
  delegate :name, to: :user, prefix: true, allow_nil: true
end

<%= @vote.user_name %>

delegateメソッドは、引数でtoが必須になっております。toで関連するモデルを指定してあげましょう。
prefixの指定がtrueの場合、@vote.user_nameの呼び出しが可能になります。もしも、falseの場合は、@vote.nameという呼び出し方になってしまい、これでは、「voteオブジェクトのname」を呼び出しているようになってしまい、本末転倒です。加えて、prefixにはboolean値以外にもstringが指定できて、例えば、prefix: :authorとすると、@vote.author_nameといった形で呼び出せるようになります。
また、allow_nilというオプションもあるのですが、これは単純にnilが入ってきても良いかという指定です。

Move code into helper

これはviewで書いてしまっているoption_tagが少し複雑になってしまっているので、それだったらhelperに移動しようという警告です。

<%= select_tag('favorite_color',
               options_for_select([
                 ['赤', red],
                 ['青', blue],
                 ['黄', yellow],
                 ['緑', green]]) %>

<%= select_tag('favorite_color', options_for_select_color) %>

# app/helpers/color_helper.rb
def select_color_tag
  options_for_select([["赤", red],
                      ["青", blue],
                      ["黄", yellow],
                      ["緑", green]])
end

みたいな感じで、移行するのをおすすめされます。デフォルトだと、optionが3つ以上だと警告を示します。ただこれは、複雑になる場合を想定しているので、あくまで「optionの中身をhelperに見に行かなければいけない」というところに注意する必要があるでしょう。そのoptionの指定が複数ある場合は、統一する必要があるかもしれませんが、単一であるならば、viewのみで書いておくのが良いのではないかと思います。

Remove empty helpers

何も使ってないhelperは削除しなさい！ということですね。削除しましょう。
また、rails generate controller hogehogeとかでhelperファイルが生成するのを嫌がるのであれば、config.generators.helper = falseをconfig/application.rbに追加してあげると、生成されずに済みます。

Replace Complex Creation with Factory Method

本来Modelがやるべき処理をControllerでやってしまっているとこういう警告が出るでしょう。

悪い例がこちらです。

class VotesController < ApplicationController
  def create
    @vote = Vote.new
    @vote.user_id = @user.id
    @vote.reason = params[:reason]
    @vote.type = 0 
    @vote.public_flag = params[:public_flag]
    @vote.save
  end 
end

これはとてもよくありません。なぜかController内で、モデルオブジェクトの生成から値のセットを行っております。データに関する処理はModelがやるべきですね。

class VotesController < ApplicationController
  def create
    @vote = Vote.new_by_user(params, @user.id)
    @vote.save
  end
end

class Vote < ActiveRecord::Base
  def self.new_by_user(data, user_id)
    vote = self.new
    vote.user_id = user_id
    vote.reason = data[:reason]
    vote.type = 0
    vote.public_flag = data[:public_flag]
    vote
  end
end

こういった形の方がよいかもしれません。またnewを呼び出してその後にメソッドを何度も呼び出しているようなコードになってしまっているので、newの引数として値を入れてしまうかattributesメソッドを使用してやるかが良いと思います。

class Vote < ActiveRecord::Base
  def self.new_by_user(data, user_id)
    self.new(user_id: user_id,
             reason: data[:reason],
             type: 0,
             public_flag: data[:public_flag])
  end
end

# もしくは
class Vote < ActiveRecord::Base
  def self.new_by_user(data, user_id)
    vote = self.new
    vote.attributes = {user_id: user_id,
                       reason: data[:reason],
                       type: 0,
                       public_flag: data[:public_flag]}
  end
end

Use query attribute

これはnilか""か[]のときにtrueを返すblank?やfalseを返すpresent?メソッドを使用していると出る警告です。ただし、ActiveRecordを通して取得してきたメソッドに対してのみです。例えば、このようなかたちにします。

# before
<% if @user.email.present? %>
  <p><%= @user.email %></p>
<% end %>

# after
<% if @user.email? %>
  <p><%= @user.email %></p>
<% end %>

すっきりしますが、「値が入ってるかどうか」を知りたければ、明示的にpresent?などを書いてあげる方が親切かもしれないと個人的には思います。
ただquery attributeを使用すると、0もfalseになるらしく、0か1で判定しているテーブルには使うと良いかもしれません。

Simplify render in views/controllers

レンダリングメソッドがあるのですが、シンプルに書きましょうということです。

# before
<%= render :partial => "header" %>
<%= render :partial => 'posts/post', :object => @post %>
<%= render :partial => 'post', :collection => @posts %>
<%= render :partial => 'comments/comment', :locals => { :parent => post } %>

# after
<%= render "header" %>
<%= render @post %>
<%= render @posts %>
<%= render 'comments/comment', :parent => post %>

また、例はViewで使用するときのものですが、controllerにも当てはまるような内容なので、controllerでrenderメソッドを使うときも省略しましょうということですね。

Restrict auto-generated routes

使ってないroutingの設定を省きましょうということです。
resoucesを使用して、7種類のrouting(index, new, create, show, edit, update, destroy)が生成されますが、もしもどれか使用していない場合は、config/route.rbの中で除外してしまいましょう。except: ["edit", "show"]のようにして指定してあげましょう。
また7種類なので4個以上除外してしまうような場合は、only: ["create", "destroy"]といったかたちで、使用するものだけを指定してあげるのがよいでしょう。

class VotesController < ApplicationController
  def index; end
  def new; end
  def create; end
  # def show; end
  # def edit; end
  # def update; end
  def destroy; end
end

Rails.application.routes.draw do
  resources 'users', :except => [:show, :edit, :destroy]
end

まとめ

あくまでRailsでの良い書き方を指摘してくれるGemですので、プロダクトを作る上で便利かどうかはわかりませんが、チームでの開発を行う中でこのようなGemを導入しておくと、コードに統一感が出る他、余計なものを残さずに済むのでよいのではないでしょうか。