重複したコードを綺麗にリファクタリングしてみた

今回の内容

作成していたポートフォリオがやっと形になり、メンターの方にコードレビューをしてもらったときの気付きを備忘録も兼ねて紹介しています。

該当箇所の画面

環境

macOS Big Sur 11.2.3
ruby: 2.7.2
rails: 6.1.3
テンプレートエンジン: slim
レイアウト: bootstrap4

前提

• bootstarpの導入は省いています

リファクタリング前のコード

bootstap4のclassを使用してページ毎にボタンの色を変更しています。

btn-primary： 青色
btn-light： 灰色

app/views/users/travel_records.html.slim

.user-detail.p-4
  = render 'profile_detail', user: @user
  .btn-group.btn-group-lg.d-flex.mb-4
    = link_to travel_records_user_path(@user.id), class: 'btn btn-block btn-primary text-white flex-fill'
      | はなたび記録
    = link_to favorites_user_path(@user.id), class: 'btn btn-block btn-light flex-fill'
      | お気に入り
    = link_to likes_user_path(@user.id), class: 'btn btn-block btn-light flex-fill'
      | いいね記録
    = link_to followings_user_path(@user.id), class: 'btn btn-block btn-light flex-fill'
      | フォロー
    = link_to followers_user_path(@user.id), class: 'btn btn-block btn-light flex-fill'
      | フォロワー
  .px-3
    .user-detail__info
     | はなたび記録 (#{@user.travel_records.count})

上記では、はなたび記録ページを表示しているため、該当リンクにbtn-primaryを付与し、ボタンの色を青色に、その他リンクにはbtn-lightを付与し、灰色にしています。

他ページでも同様のコードを記述しており、差異はボタンの色を変えるために付与しているclassのみとなっています。

リファクタリングの手順

• 重複コードをパーシャル化

• 部分テンプレートのローカル変数に値を付与

パーシャル化

ファイル名は分かりやすく_menuとしています。

app/views/users/_menu.html.slim

.btn-group.btn-group-lg.d-flex.mb-4
  = link_to travel_records_user_path(@user.id), class: "btn btn-block btn-#{ active == 'travel_record' ? 'primary' : 'light' } flex-fill"
    | はなたび記録
  = link_to favorites_user_path(@user.id), class: "btn btn-block btn-#{ active == 'favorite' ? 'primary' : 'light' } flex-fill"
    | お気に入り
  = link_to likes_user_path(@user.id), class: "btn btn-block btn-#{ active == 'like' ? 'primary' : 'light' } flex-fill"
    | いいね記録
  = link_to followings_user_path(@user.id), class: "btn btn-block btn-#{ active == 'follow' ? 'primary' : 'light' } flex-fill"
    | フォロー
  = link_to followers_user_path(@user.id), class: "btn btn-block btn-#{ active == 'unfollow' ? 'primary' : 'light' } flex-fill"
    | フォロワー

今回は、条件演算子（三項演算子）を使用することで条件式の結果によって画面の切り替えを行なっております。

{ パーシャルに渡す変数 == 変数の値 ? 真の時の値 : 偽の時の値 }

class: "btn btn-block btn-#{ active == 'travel_record' ? 'primary' : 'light' } flex-fill"

上記を例に説明すると、パーシャルに渡す引数の値が'travel_record'の場合は'primary'という文字列を渡し、そうでない場合は'light'という文字列を渡すといった感じです。

※ 式展開を使用するため、classはダブルコーテーション("")で囲い、付与する値はシングルコーテーション('')で囲っています。

リファクタリング後のコード

部分テンプレートのローカル変数(active)に'travel_record'という文字列を渡すことで、calssにbtn-primaryが付与され、スタイルが適用されるようになります。

app/views/users/travel_records.html.slim

.user-detail.p-4
  = render 'users/profile_detail', user: @user
  = render 'users/menu', active: 'travel_record'
  .px-3
    .user-detail__info
     | はなたび記録 (#{@user.travel_records.count})

※ お気に入りページの場合、ローカル変数の値は'favorite'に変更します。

最後に

以上が重複したコードのリファクタリングでした。
初めてコードレビューをしてもらい、終始「こんな記述方法が、、あるのか、」とワクワク状態が続いていました笑
他にも色々と助言してもらい、多くの学びを得ることができ、今後に活かしていきたいと感じた今日この頃です！

間違っている箇所や分かりづらい箇所が多々あるかと思います。
その際は、気軽にコメントいただけれると幸いです。

最後までご覧いただき、ありがとうございました!