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

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

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

### Active Recordはeachの外で使う

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

```ruby
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から出してみましょう。

```ruby
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以外にも有用なメソッドがたくさんあるのでそちらを使い可読性をあげましょう。

```ruby
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)](https://docs.ruby-lang.org/ja/latest/class/Enumerable.html#I_COLLECT)

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

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

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

### ifのネストを浅くする

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

#### 例

```ruby
# このメソッドは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文のネストをといてみます。

```ruby
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に書くことが妥当なので書き写してみましょう。

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

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

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

これで完了です。

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

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

```ruby
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_* って必要？ - ネットの海の片隅で](http://osa.hatenablog.com/entry/good-bye-before-action-setter)
こちらのページに書かれている通りです。
set_*に限らずbefore_actionを大量に使うと暗黙的な順番が発生してしまうため多用はやめましょう。

修正前

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

  def show
  end

  private

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

修正後

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

もしくは

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

  private

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

### 横着しない

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

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

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

```ruby
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を学ぶという人の助けになれば幸いです。