1. はじめに
kekiと申します。
スクールでのアプリ制作にて「pictures#index画面でもお気に入り/お気に入り解除機能を作りたい！」と思い実装にチャレンジしてみました。ところが思っていた以上に難しく、特にidの抽出に苦労した上に改善部分も見受けられる結果となりましたので、記録を残しておこうと思いました。
最終的にこういった感じの見た目になりました。
2. 実装内容の想定と手順
以下の条件でViewファイルのみでのコーディングを試みました。
- pictures#index画面にて、お気に入り機能を追加実装する
- アソシエーションはuser(1) ⇔ favorites（多） ⇔ picture（1）の関係で構築済み
- pictures#showで実装済みのお気に入り機能を参考にコーディングする
- favorites#destroyアクションはpictures#showにて問題なく挙動していたため、今回はコントローラ内を変えずindex.html.erbの方を変えることとする
- 具体的に以下のようなコードを想定する
- 「投稿写真のuser_idとcurrent_userのidが一緒でない（つまり自分以外のユーザーの投稿に対しての）場合に、」
- 「もしお気に入りのidが存在していたら（お気に入りを既にしていたら）、」
- 「お気に入り解除する」
- 「見つけられなかったら（お気に入りをまだしていなかったら）」
- 「お気に入りをする」
3. 実装まで
3-1. include?メソッドを試す
まずは上記の5. のコードを以下のように書いてみました。
<% @pictures.each do |picture| %>
<tr>
#省略
<% unless picture.user_id == current_user.id %>
<% if @favorites.pluck(:id).include?(picture.favorites.pluck(:id)) %>
<td><%= link_to 'お気に入り解除する', favorite_path(id: picture.favorite_users.pluck(:id)), method: :delete, class: "btn btn-danger" %></td>
<% else %>
<td><%= link_to 'お気に入りする', favorites_path(picture_id: picture.id), method: :post, class: "btn btn-outline-danger" %></td>
<% end %>
<% end %>
#省略
</tr>
<% end %>
binding.pryでpicture.favoritesの中身を確認したところ、アソシエーションで結びついているのでfavorite.idが含まれていたことから思いつきました。
pluckメソッドは「引数で指定したカラムの値をすべて抽出して、配列に返してくれるメソッド」です。
include?メソッドは「引数で指定した値が含まれている場合は真、無ければfalseを返すメソッド」です。
それを踏まえ「if文内で@favorites（全てのお気に入りのデータ）からfavoriteのidのカラムを抽出して、その中にeach文で一つずつ発行されたpictureに対し、picture.favoritesとして含まれたfavorite.idを引き出そう」と考えました。
ですがこれはif文内が全てfalseになります。
実際のbinding.pryでのログは以下のようになりました。
[3] pry(#<#<Class:0x0000000130be6550>>)> @favorites.pluck(:id)
=> [24, 26]
[4] pry(#<#<Class:0x0000000130be6550>>)> picture.favorites.pluck(:id)
(0.6ms) SELECT "favorites"."id" FROM "favorites" WHERE "favorites"."picture_id" = $1 [["picture_id", 1]]
↳ (pry):4
=> [24]
[5] pry(#<#<Class:0x0000000130be6550>>)> @favorites.pluck(:id).include?(picture.favorites.pluck(:id))
CACHE (0.0ms) SELECT "favorites"."id" FROM "favorites" WHERE "favorites"."picture_id" = $1 [["picture_id", 1]]
↳ (pry):5
=> false
if文左辺の[24, 26]という配列に対して、include?で右辺の[26]という配列と比較しています。
一見trueを返しそうなものですがfalseです。何故でしょうか？
よりわかりやすいinclude?の例を載せます。
[1] pry(#<FavoritesController>)> [24, 26].include?(24)
=> true
[2] pry(#<FavoritesController>)> [24, 26].include?[26]
ArgumentError: wrong number of arguments (given 0, expected 1)
from (pry):6:in `include?'
上の場合include?に対して(24)と引数を渡したらtrueが返りました。
一方で下の場合は[24]と渡すとfalseになりました。
つまりinclude?メソッドの引数は配列そのものを渡すと引数がうまく渡されず、結果falseになることがわかりました。というわけでinclude?ではなく、別のメソッドを使うことにしました。
3-2. any?メソッドを使う
メソッドを変えれば上手くいきそうだったので、スクールのメンターにany?メソッドを紹介してもらいました。
any?メソッドは二つの配列に対し、以下のような挙動をします。
array_1 = [1,2,3]
array_2 = [3]
(array_1 & array_2).any?
=>true
これならinclude?では上手くいかなかった配列出力同士の検索でも使えそうです。
if文内を以下のように書き換えました。
<% if (@favorites.pluck(:id) & (picture.favorites.pluck(:id))).any? %>
<td><%= link_to 'お気に入り解除する', favorite_path(Favorite.find_by(picture_id: picture.id, user_id: current_user.id)), method: :delete, class: "btn btn-danger" %></td>
<% else %>
<td><%= link_to 'お気に入りする', favorites_path(picture_id: picture.id), method: :post, class: "btn btn-outline-danger" %></td>
<% end %>
これでifの分岐が「お気に入りしてたら真、まだしていなかったら偽」とうまく通りました！
ただ他のメンターにも聞いてみたところ、さらに別の方法も見つけたのでそちらも試してみることにしました。
3-3. find_byメソッドを使う（採用）
今回if内で欲しいのは該当のお気に入りのid一つなので、find_byメソッドでそのidが一つ抽出できればやはり成功します。
実際に以下のコードが完成し、最終的に採用しました。
<% unless picture.user_id == current_user.id %>
<% if Favorite.find_by(picture_id: picture.id, user_id: current_user.id) %>
<td><%= link_to 'お気に入り解除する', favorite_path(Favorite.find_by(picture_id: picture.id, user_id: current_user.id)), method: :delete, class: "btn btn-danger" %></td>
<% else %>
<td><%= link_to 'お気に入りする', favorites_path(picture_id: picture.id), method: :post, class: "btn btn-outline-danger" %></td>
<% end %>
<% end %>
if文内の説明をします。
まずFavorite.find_byとして、お気に入りのレコードすべてを呼び出します。その中から該当の一つのお気に入りのidを特定するため、二つのキーを指定します。一つの写真に対して一人のユーザーがお気に入りできる回数は基本的に一回のみなので、写真一枚のidとログインユーザーのidでお気に入りのidも絞れる、という仕組みです。
こちらも成功し、またそれに加えお気に入り解除時のfavorite_path(id)の引数にも再利用できました！
個人的には違うコードが二つあるよりは同じコードが二回動いている方が、Restfulでは無いけど可読性が高いかな、と考えこちらを最終的に採用しました。
以上で実装は完了です！
ただし、このfind_byメソッドを使ったコードにはいくつかの問題点があります。
4. 問題点
さて無事実装し、挙動も問題なく見えるfind_byメソッドでの方法ですが、以下の問題点が浮かびました。
4-1. 処理が重くなる
まず一番の問題は処理が重くなる可能性が高いことです。
最終的に実装したindex.html.erbの中では、Favorite.find_by( )が二回行われています。ログを見ると、一つのお気に入りを検索するのに毎回find_byの検索が走っている事がわかります。しかも今回は二回find_byが走っているので、二度find_byが走ります。
現状は速度にほぼ影響は出ていませんが、もしお気に入り数が数件ではなく数百件だったら…塵も積もりてで処理はどんどん重くなっていくでしょう。
4-2. ビューファイルに役割を持たせすぎている
上記の理由と連なりますが、そもそもRailsは「役割を分担させる」ことが設計思想上、重要視されています。
言い換えれば、ビューファイルはあくまで画面上の表記についてのコードに絞り、複雑な処理や計算はコントローラやヘルパーメソッドに投げるべき、という理想があります。
今回の私の採用したコードでは真逆で、ビューファイルの中で全て計算をしています。いわゆる「アンチパターン」というコードになってしまっています。
このあたりのビューファイルの理想設計やアンチパターンについては、以下の記事がわかりやすかったです。気になる方はぜひ参考にしてみてください。
フリーランス歴20年の強強エンジニアからのガチコードレビュー集(include?の代替案などドンピシャなヒントがあります)
5. 感想
このようにindex画面上にお気に入りの表示を出す事には成功しましたが、現状の私のコードではデータが膨大になった時に処理が重くなる可能性が高く、改善点が残る結果となりました。
showでの詳細画面では比較的容易だったお気に入り機能ですが、index画面だとこうも複雑になるんだなと自分で実装してみて大変驚きました。
では実際のSNSのTLはそこまで重く無いのは何故なのか？と思いメンターに聞いてみたところ、実際はJavaScriptの非同期設定などが複合的に駆使されており別の角度からの処理軽減を実現している（と思う）とのことです。
また私たちがよくやっているSNSのTLのいいね機能は、一見簡単に見えても実際は様々な軽量化の工夫があるんだなぁ…と痛感しました。
総じて今まで感じたことのなかった処理速度、コードの重量についてを新たに考える良い機会になりました。
7. 終わりに
結論をまとめますと、index画面のお気に入り機能の実装はany?メソッドとfind_byメソッドの二つで成功しましたが、特にfind_byメソッドでレコード検索をした場合ビューファイルが重くなりがちという改善点に気付けました。
今後技術力が上がったら改めてこの辺りの改善に取り組んでみたいです。
もし「その条件だったらこんなメソッド使うといいよ！」や「こんなコントローラだと簡単だよ！」といったアイディアがありましたら、ぜひコメント欄にてアドバイス頂けますと幸いです。
長文読んでくださりありがとうございました。