「カレントユーザーの情報のみ」かつ「DBでカウントした数」をモデルに記述したい。（コントローラーの肥大化を解消したい）

Question

##実現したいことについて

• オリジナルアプリを作っています。
• 現在、チェックボタンにチェックをし、上の「SAVE」ボタンを押すとenumを使ってデータベースに保存されるようになっています。（チェックしたものは「１」、チェックがないものは「０」として、保存される）
• 下の丸い部分について、「カレントユーザーの情報のみ」かつ「今までのカウント合計をカテゴリーごとに出す」実装を行っています。
• 今回悩んでいることは、コントローラーが肥大化しており、カウント数を取得する部分についてモデルに記述したいものの、実装が上手くいかないことです。

##現在のコントローラー（カウント数を取得することも記述しており、肥大化している・・改善できないか？？????）

class HabitsController < ApplicationController
  before_action :authenticate_user!

  def index
    @habit = Habit.new
    @count_1 = Habit.includes(:habits).where(user_id: current_user.id).where(count_1: 1).size
    @count_2 = Habit.includes(:habits).where(user_id: current_user.id).where(count_2: 1).size
    @count_3 = Habit.includes(:habits).where(user_id: current_user.id).where(count_3: 1).size
    @count_4 = Habit.includes(:habits).where(user_id: current_user.id).where(count_4: 1).size
    @count_5 = Habit.includes(:habits).where(user_id: current_user.id).where(count_5: 1).size
    @restart_1 = Habit.includes(:habits).where(user_id: current_user.id).where(restart_1: 1).size
    @restart_2 = Habit.includes(:habits).where(user_id: current_user.id).where(restart_2: 1).size
    @restart_3 = Habit.includes(:habits).where(user_id: current_user.id).where(restart_3: 1).size
    @restart_4 = Habit.includes(:habits).where(user_id: current_user.id).where(restart_4: 1).size
    @restart_5 = Habit.includes(:habits).where(user_id: current_user.id).where(restart_5: 1).size
  end

  def new
  end

  def create
    habit = Habit.create(habit_params)
  end

  private

  def habit_params
    params.require(:habit).permit(:count_1, :count_2, :count_3, :count_4, :count_5, :restart_1, :restart_2, :restart_3, :restart_4,:restart_5).merge(user_id: current_user.id)
  end
end

##現在のモデル（アソシエーションを組んでいる）

# ユーザーモデル
class User < ApplicationRecord
 has_many :habits
end

# habitモデル
class Habit < ApplicationRecord
  belongs_to :user
end

##現在のマイグレーションファイル

class CreateHabits < ActiveRecord::Migration[6.0]
  def change
    create_table :habits do |t|
      t.references :user,null: false, foreign_key: true
      t.boolean    :count_1    
      t.boolean    :restart_1  
      t.boolean    :count_2    
      t.boolean    :restart_2  
      t.boolean    :count_3    
      t.boolean    :restart_3  
      t.boolean    :count_4    
      t.boolean    :restart_4  
      t.boolean    :count_5    
      t.boolean    :restart_5  
      t.timestamps
    end
  end
end

##調べたこと（仮説と実証しているが上手くいかないこと）

まずはコントローラーの記述をリファクタリングできないか？と考えました。

###仮説１　N+1問題を解消するために、includesを使用する

ビフォー

    @count_1 = current_user.habits.where(count_1: 1).size

アフター（現在のコード）

    @count_1 = Habit.includes(:habits).where(user_id: current_user.id).where(count_1: 1).size

includesを使用したとしても、カテゴリーごとにcountやsizeをしようとすると、複数行になってしまう。

###仮説２　ハッシュを使い、複数行になってしまっているカラムを一行にする

こちらの記事を参考に、下記の通り実装するも、データを取得することができない。

 @habits = current_user.habits.includes(:habits{count_1: 1, count_2: 1, count_3: 1, count4: 1, count_5: 1, restart_1: 1,（省略）}).size

（補足）その後、ハッシュを使うのは、ネストされているとき（throughオプションなどを使用して、複数アソシエーションを組んでいるとき）時の話ではないか？と気がつきました。

---

ここからは、そもそもコントローラーとモデルの役割ってどういう立ち位置？と調べ直しました。その結果、DBのデータを扱うメソッド（検索メソッド、レコード取得メソッド、レコード更新メソッド）などはモデルに記述するのが適切では？と思いました。

###仮説３ モデルに記述する

# モデル(まずは一つ目のカテゴリーを取得できるか実験）
def self.count_habit
 Habit.includes(:habits).where(user_id: current_user.id).where(count_1: 1).size
end

# コントローラー
  def index
    @habit = Habit.new
    Habit.count_habit
　end

としてbinding.pryを使用したところ、下記のようなエラー。

NameError: undefined local variable or method `current_user' for #<Class:0x00007fc2023e19f0>
Did you mean?  current_scope

# カレントユーザーを外すと上手くいく
[3] pry(Habit)> Habit.includes(:habits).where(count_1: 1).size
  CACHE  (0.0ms)  SELECT COUNT(*) FROM `habits` WHERE `habits`.`count_1` = TRUE
  ↳ (pry):3:in `count_habit'
=> 146

という状況です・・。

そこから、カレントユーザーに関しては、コントローラーで使えるものであって、モデルでは使用できないという事を知りました。

こちらの記事を参考に、application_controller.rbのビフォーアクションで、「カレントユーザーを前提にする方法」も試してみましたが、思った通りのデータを取得することができません。

##教えていただきたいこと

• そもそも、上記のコントローラーの状況は良くないのでモデルに書きたいという方向性は合っているのでしょうか？

（初めは、ビューで<%= "通算#{@user.where(count_3: 1).count}回" %>のような表現をしていたのですが、今後JavaScriptで非同期通信を行いたい時に困ると感じ、コントローラーにまず移しました。）

• そして「カレントユーザーの情報のみ」かつ、「今までのカウント合計をカテゴリーごとに出す」モデルの記述方法がわからず苦戦しております????記述方法のヒントだけでも、教えていただけると嬉しいです????

Answer 1

一般的には「モデルの情報で処理できるものはモデルに書く」というのは正しいと思っています。current_userは引数で渡せばよいです。
ただこの場合にそれがリファクタリングとして意味を持つか（すっきりするか）はちとわからない。

indexにかかれているような　一部だけ違って他は同じ、というとき私はこんなことします
@count = %w[count_1 count_2 count_3 count_4 count_5 restart_1 restart_2 restart_3 restart_4 restart_5].map {|clm| [clm, Habit.where(user_id: current_user.id, clm => 1).count ]}.to_h
これで { "count1" => 12 , ,,,,} が得られます。
Habit の検索に includes(:habits) は意味が無いです。多分無視されてます。
%w[count_1 ..... rstart_5] も冗長なので
%w[count_ rstart_].product(%w[1 2 3 4 5]).map(&:join) としたいけど、謎が深まるから良くはないかな。半分になるだけだからやめときますか。

Comments

[cherry2020]

winterboumさん、回答ありがとうございます。

モデルの情報で処理できることはモデルに書くことや、カレントユーザーをモデルで実装する場合には、引数で渡す方法があることについて勉強になりました。

そして１０行あった文がwinterboumさんのおかげで１行で実装できました！！！（；ω；）
本当にありがとうございます！！！！！！

コントローラーは
「@count = %w[count_ restart_].product(%w[1 2 3 4 5]).map(&:join).map {|clm| [clm,Habit.where(user_id: current_user.id, clm => 1).count]}.to_h」
ビューは
「<%= "通算#{@count["count_1"]}回" %>」で呼び出す

でブラウザ上も反映しました。

配列を%W記法で表現することや、productメソッドやmapメソッドについて、使おうという発想がなかったたです。美しいコードに感動しました。
冗長なコードをすっきりまとめられる知識を身に付けたいと思いました。

winterboumさん本当にありがとうございました。

[winterboum]

甘い！
viewも同じ所と違う所を分け
<% [ ここに coimt_1 とか並べる].each_with_index do |colmn, idx| %>
して、
label[idx] と "通算#{@count[colmn]}回"

[cherry2020]

winterboumさん、有難うございます！
教えていただいたものの、実装に苦慮して時間がかかってしまいました＞＜

結果、<% @counts.values.each do |val| %>と<%= "#{val}回" %>で
思い通りの文字を表示させることができました＾＾
（当初の５分の１の記述量になりました！！！）

winterboumさんが教えてくださった「_with_index」をつけると
順番を併記することも出来るのですね。

ビューにおいて要素に関しても繰り返し表示させることが出来ると、大変勉強になりました。
解決後もフィードバックいただいてありがとうございました！