【Rails】通知機能をリファクタリングしてみた

プログラミングスクールに通っています。
先日ポートフォリオのフィードバックが返って来た際に、通知機能のリファクタリングをした方が良いとの指摘を受けたので、修正方法を記事にします。
通知機能に関しては以下の記事を参考にさせて頂きました。
https://qiita.com/nekojoker/items/80448944ec9aaae48d0a

#修正方法
```ruby:cook.rb
  def create_notification_like!(current_user)
    already_like_check = Notification.where(
      [
        "visitor_id = ? and visited_id = ? and cook_id = ? and action = ? ",
        current_user.id, user_id, id, 'like',
      ]
    )
    if already_like_check.blank?
      notification = current_user.active_notifications.new(
        cook_id: id,
        visited_id: user_id,
        action: 'like'
      )
      if notification.visitor_id == notification.visited_id
        notification.is_checked = true
      end
      notification.save if notification.valid?
    end
  end
```
投稿した料理に対して「いいね」がされた際に通知が行われるようにしています。
今回はモデルに定義したcreate_notification_like!メソッドをリファクタリングしていきます。

上記では
- already_like_check が blank だった場合
  - notification.visitor_id と notification.visited_id が一緒だった場合
  - notification が valid? だった場合
と条件が3つ出てきます。しかし、 `already_like_check` が `blank` じゃなかった場合の処理は出てきません。
つまり、`already_like_check` が `blank` じゃなかった場合 は例外的なパターンとして処理をその時点で中断してあげると良いということになります。
この考え方を適用すると下記のように実装できます。

```ruby:cook.rb
class Cook < ApplicationRecord
  def create_notification_like!(current_user)
    already_like_check = Notification.where(
      [
        "visitor_id = ? and visited_id = ? and cook_id = ? and action = ? ",
        current_user.id, user_id, id, 'like',
      ]
    )
    return unless already_like_check.blank? #これを追加
    notification = current_user.active_notifications.new(
      cook_id: id,
      visited_id: user_id,
      action: 'like'
    )
    if notification.visitor_id == notification.visited_id
      notification.is_checked = true
    end
    notification.save if notification.valid?
  end
end
```
早期リターンという手法でifのネストを避けることができました。
（早期リターンとは簡単に言うと、「条件に合致しない場合は処理をそこでストップさせる」ことです。今回のケースでは`already_like_check` が `blank` じゃなかった場合、`return unless already_like_check.blank?`以下の処理を実行しないということです。）
次は メソッドの抽出 という方法を行い処理をみやすくしてみましょう。
メソッドの抽出 というのは処理の塊をメソッドに切り出してあげることです。
今回抽出できそうな処理は下記になります。

```ruby
notification = current_user.active_notifications.new(
    cook_id: id,
    visited_id: user_id,
    action: 'like'
  )
if notification.visitor_id == notification.visited_id
  notification.is_checked = true
end
notification.save if notification.valid?
```
この処理は、
- notification.visitor_id と notification.visited_id が一緒だった場合
- notification が valid? だった場合 
という2つの条件が含まれており、 `create_notification_like!` メソッドの中にあるとやや処理が複雑になってしまいます。
`Notification` クラスから作成したインスタンスに対する処理の実行ですので、 `Notification` クラスにインスタンスメソッドを作成してあげると良いでしょう。

```Ruby:notification.rb
def update_is_checked_and_save
  if visitor_id == visited_id
    is_checked = true
  end
  save if self.valid?
end
```
さらに、後置ifを活用して以下のようにします。

```Ruby:notification.rb
def update_is_checked_and_save
  is_checked = true if visitor_id == visited_id
  save if self.valid?
end
```

そして、このメソッドを `create_notification_like!` メソッド内で呼び出します。

```Ruby:cook.rb
def create_notification_like!(current_user)
  already_like_check = Notification.where(
    [
      "visitor_id = ? and visited_id = ? and cook_id = ? and action = ? ",
      current_user.id, user_id, id, 'like',
    ]
  )
  return unless already_like_check.blank?
  notification = current_user.active_notifications.new(
    cook_id: id,
    visited_id: user_id,
    action: 'like'
  )
  notification.update_is_checked_and_save
end
```
これで最初よりかはスッキリとしたと思います。

#最後に
あくまで一例ですので、もっとわかり易い方法があれば教えて下さい！
以上です！ありがとうございました！