なぜ書くか

これまたコードレビューの際にご指摘いただきました。
備忘録と、コツコツとプロの開発者としての思考を身につけるためにアウトプットさせていただきます。

脱「動けばいい」精神

まず、前提としては、RailsはApiでのやり取り専用のコントローラ。
iOSアプリケーションからリクエストを受け取り、それぞれのリクエストに応じた処理が実行されます。

実際に問題となった機能を実装する前は、コントローラは下記の様な状態でした。（コードは多少変更しています。）

module Api
  module V1
    class ProductController < ::Api::ApplicationController
      protect_from_forgery except: [:create]

      def index
        render json: Product.trader_filter(params[:trader_id])
      end

      def create
        render json: Product.new(product_params).save
      end

      private

      def product_params
        params.permit(:name, :price)
      end
    end
  end
end

しかしここでiOS側でのとあるページの都合で「その日、当日分の商品情報だけ欲しいな...」といったことが発生しました。（なんの当日分かは置いといて）
すかさず、Railsアプリケーションのコードを追加し、書いたコードは下記の様なコードです。

module Api
  module V1
    class ProductController < ::Api::ApplicationController
      protect_from_forgery except: [:create]

      def index
        render json: Product.trader_filter(params[:trader_id])
      end

      def create
        render json: Product.new(product_params).save
      end

      def acquisition_of_product_data_on_the_day
        render json: Product.trader_release_date_filter(params[:trader_id], params[:release_date])
      end

      private

      def product_params
        params.permit(:name, :price)
      end
    end
  end
end

ルーティングにコレクションを使用し、七つのアクション以外のアクションを定義し、コントローラでモデルのスコープを呼び出しています。
この時は「もうindexは使っちゃってるしなぁ」と安直にアクションを追加し、これでいいと思っていました。

しかし、先輩にレビュー依頼出したところ、ここが指摘されました。

理由としては、検索条件を追加するたびにいちいちアクションを追加していたら、コントローラのメソッドの数がえげつないことになり、いわゆる「fat Controller」になってしまいメンテナンスが難しくなってしまうということでした。

まさに「動くからええやん...」気分で書いたコードでした。
しかし実際に本当にその通りで、検索の内容が変わるたびに「get」のリクエストを増やしまくっていたのでは、いつか維持できなくなります。
「将来的にも追加はそこまでないからいいのでは？」と思ったりもしたのですが、とにかく同じようなメソッドを何個もコントローラーに書くのはスマートではないと。

ではどうするか？

本来コントローラは左から右へ処理、命令を流すだけの役割です。
なので、当日分のデータを返したいのかそうでないのか？といったこともコントローラに考えさせるべきではありません。

なのでindexで、全体取得、本日分のみの取得のリクエストも受け取り、モデルに判断させます。

module Api
  module V1
    class ProductController < ::Api::ApplicationController
      protect_from_forgery except: [:create]

      def index
        render json: Product.trader_filter(params[:trader_id], params[:release_date])
      end

      def create
        render json: Product.new(product_params).save
      end

      private

      def product_params
        params.permit(:name, :price)
      end
    end
  end
end

最初との違いは、indexメソッドに渡すparamsの中身を追加しただけです。つまり取得したい日付のデータですね。

そしてモデルのスコープは以下のように実装します。

scope :trader_filter, ->(trader_id, release_date) do
  if release_date.nil?
    #全件検索の処理
  else
    #当日分のデータ取得の処理
  end
end

これで、コントローラにアクションを増やすことなく、同じ動きを実現できました。
コントローラから、スコープを見に行くだけで、どのようなデータを返すようになっているかも一発で分かるので可読性もこちらの方が高いと思います。