railsで初心者が開発する際に気をつけておきたいこと #Rails

railsを始めてそろそろ一年になるので、開発を進める上で先輩方に言われたことや経験則をまとめました。
初歩的なことを中心にまとめましたので初心者の方々はどうぞ参考にしてください。

Active Recordはeachの外で使う
each以外のメソッドを使う
ifのネストを浅くする
scopeは必ずActiveRecord::Relationを返すようにする
before_action :set_*を使わない
横着しない

Active Recordはeachの外で使う

例

任意のidを元にUserを検索し、検索されたUserのnameを加工したものをarrayとして返すメソッドを作るとします。

user_ids = [1,2,3,5,6,7,8,11]
values = []

user_ids.each do |user_id|
  user = User.find(user_id)
  name = user.name
  ## いろいろな処理
  values.push(name)
end

return values

このコードではeachを使い複数のidを検索しnameを取り出しているようです。
ここでログを見てみると

User Load  SELECT  "users".* FROM "users" WHERE "users"."id" = 1
User Load  SELECT  "users".* FROM "users" WHERE "users"."id" = 2
User Load  SELECT  "users".* FROM "users" WHERE "users"."id" = 3
User Load  SELECT  "users".* FROM "users" WHERE "users"."id" = 4
User Load  SELECT  "users".* FROM "users" WHERE "users"."id" = 5
User Load  SELECT  "users".* FROM "users" WHERE "users"."id" = 6
User Load  SELECT  "users".* FROM "users" WHERE "users"."id" = 8
User Load  SELECT  "users".* FROM "users" WHERE "users"."id" = 11

となりクエリをなんども発行しているのがわかります。
なのでまずはUser.findをeachから出してみましょう。

user_names = User.where(id: [1,2,3,5,6,7,8,11]).pluck(:name)
values = []

user_names.each do 
  ## いろいろな処理
  values.push(name)
end

return values

まだ改良の余地がありますが、ログを見てみると一つのクエリで完了していることがわかります。

User Load  SELECT "users".* FROM "users" WHERE "users"."id" IN (1,2,3,5,6,7,8,11)

each以外のメソッドを使う

先ほどのコードではeachを使っていました。ですが、railsにはeach以外にも有用なメソッドがたくさんあるのでそちらを使い可読性をあげましょう。

user_names = User.where(id: [1,2,3,5,6,7,8,11]).pluck(:name)

values = user_names.map do
  ## いろいろな処理
  name
end

return values

今回は要素に対してブロックを評価した結果を全て含む配列を返すmapを使ってみました。
map以外のメソッドを使いたい場合はrubyのリファレンスをみてみると良いでしょう。

module Enumerable (Ruby 2.6.0)

特にselect(ブロック)はActive Recordではないため動作が早く有用です。例えば、鈴木という名前の人のみ取りたい場合。

# findを使った場合、クエリが2回発行される。
users.where(name: '鈴木')
# selectを使った場合、クエリの発行回数を減らせる。
users.select { |u| u.name == '鈴木' }

とするといいです。
今回は単純な検索のためそれほど差は有りませんが、複雑な検索の場合は処理時間に顕著な差が現れます。

ifのネストを浅くする

コードを拡張していくとif文をネストして書いてしまうケースが多々みられます。

例

# このメソッドはcontrollerに存在している。
# ユーザー50人以上にお気に入り登録をされていたらtrue
def show
  if everybody_like_user?(key: params[:key])
    ## いろいろな処理
  end
end

private

def everybody_like_user?
  if @key.present?
    user = User.find_by(key: @key)
    if user.present?
      user.like_users.count > 50
    else
      return false
    end
  else
    return false
  end
end

このようなコードは可読性が低くバグを特定しづらい、再利用性が低い等の欠点が存在します。
なのでif文をネストしてしまったら一旦立ち止まりコードを綺麗にリファクタリングしてみましょう。
returnで返す箇所を最初に書くことでif文のネストをといてみます。

def everybody_like_user?
  return false unless @key.present?
  user = User.find_by(key: @key)
  return false if user.present?
  user.like_users.count > 50
end

if文のネストはなくなりました。まだ見にくいですね。このメソッドはcontrollerではなくmodelに書くことが妥当なので書き写してみましょう。

# models/user.rbにインスタンスメソッドとして書き写す。
def everybody_like_user?
  like_users.count > 50
end

controllerにはこのように書きます。

def show
  @user = User.find_by(key: params[:key])
  if @user&.everybody_like_user?
    ## いろいろな処理
  end
end

これで完了です。

scopeは必ずActiveRecord::Relationを返すようにする

scopeは複数のクエリをまとめるものです。

class Article < ApplicationRecord
  scope :published, -> { where(published: true) }
end

scopeはActiveRecord::Relationを返すことが望ましいです。ActiveRecord::Relationを返すメソッドはこちら

find
create_with
distinct
eager_load
extending
from
group
having
includes
joins
left_outer_joins
limit
lock
none
offset
order
preload
readonly
references
reorder
reverse_order
select
where

before_action :set_*を使わない

Rails の before_action :set_* って必要？ - ネットの海の片隅で
こちらのページに書かれている通りです。
set_*に限らずbefore_actionを大量に使うと暗黙的な順番が発生してしまうため多用はやめましょう。

修正前

class UsersController < ApplicationController
  before_action :set_user, only: %i[show]

  def show
  end

  private

  def set_user
    @user = User.find(params[:id])
  end
end

修正後

class UsersController < ApplicationController
  def show
    @user = User.find(params[:id])
  end
end

もしくは

class UsersController < ApplicationController
  def show
     @user = get_user(params[:id])
  end

  private

  def get_user(id)
    User.find(id)
  end
end

横着しない

コードを見ていると、なるべくコードを書かないようにしよう、もしくはコピペで済ませようとするあまり横着をするパターンが見受けられます。
例えばmodelの中にあるカラム[body]を加工するための処理があったとしましょう。

class Article < ApplicationRecord
  def processnaize_body
    ## いろいろな処理
    body
  end
end

こちらのメソッドを別のカラム[sub_body]にも使用したいとします。
その場合processnaize_bodyに共通化を施して対処すればいいのですが、
横着するとこうなります。

def show
  @article = Article.find(params[:id])
  body = @article.body
  # sub_bodyでprocessnaize_bodyを使いたいのでbodyに移している
  @article.body = @article.sub_body 
  # processnaize_body実行
  @article.sub_body = @article.processnaize_body
  # その後bodyを戻す
  @article.body = body
end

確かにメソッドを一つも書かずに実装できていますが、これではテストがすごく書きづらいです。こういう横着は大抵の場合、余計なコードを増やすだけなのでやめましょう。

最後に

いかがでしたでしょうか？これからrailsを学ぶという人の助けになれば幸いです。