Zenn
🙄

Fat Controllerをリファクタリングしたい！

2024/11/30に公開
Rails
Ruby
tech

はじめに

ここ数ヶ月間でRailsを使用したアプリケーションの新規機能開発を担当し、見事Fat Controllerを実装してしまいました。
そのため、振り返りとして単一責務の原則に則った実装や抽象化したクラスの作成方法など、Fat Controllerをリファクタリングしながら問題点や改善策についてまとめました。

Fat Controllerとは

まず、改めてFat Controllerとはどういった状態のものなのかについて整理します。

コントローラに置くロジックを増やしすぎたときの問題は、単一責任の原則（SRP: Single Responsibility Principle）に違反することです。つまり、処理をコントローラ内で引き受けすぎてしまっているということです。この状態になるとコード量が増えて責務を抱え込みすぎてしまいがちです。ここでいう「ファット」は、コントローラファイルのコード量も、コントローラがサポートするロジックの量も増えるということを指します。ファットコントローラは多くの場合アンチパターンと見なされます。

https://techracho.bpsinc.jp/hachi8833/2021_10_05/112108

要するに、コントローラーが過剰な責務を持ってしまい、肥大化してしまっている状態のことを言います。
結果として、色々な責務を持っているコントローラーになってしまうと、可読性の低いコード・変更に弱いコード・再利用性の低いコード になってしまいます。

そのため、Fat Controllerにならないように実装するには「単一責務の原則」の考えが大切になります。

単一責務の原則

次に、改めて単一責務の原則についても整理してみます。

クラス（オブジェクト）が担う責任は1つに限定すべきである（かつその責務は完全にカプセル化されるべきである）。

https://techracho.bpsinc.jp/hachi8833/2018_03_27/54130

つまり、1つのクラスやモジュールが複数の責務を持つのではなく、1つの明確な責務だけを担うべきという考え方になります。
この原則を意識することで、コードの可読性・保守性が向上します。

では、次に単一責務の原則についてBAD・GOODなコード例をみてみましょう。

🙅‍♂️ BAD

以下のコードはフルーツのECサイトを例として考えた際のコントローラーになります。
index, show, searchのアクションが実装されており、責務が混在してしまっています。また、少し極端ではありますが、各アクションのデータ整形のロジックが重複してしまっています。

app/controllers/api/fruits_controller.rb
class Api::FruitsController < ApplicationController
  def index
    fruits = Fruit.all
    result = fruits.map do |fruit|
      {
        id: fruit.id,
        name: fruit.name,
        price: fruit.price,
        in_stock: fruit.stock > 0
      }
    end
    render json: result
  end

  def show
    fruit = Fruit.find(params[:id])
    render json: {
      id: fruit.id,
      name: fruit.name,
      price: fruit.price,
      in_stock: fruit.stock > 0
    }
  end

  def search
    fruits = Fruit.where('name LIKE :query', query: "%#{params[:query]}%")
    result = fruits.map do |fruit|
      {
        id: fruit.id,
        name: fruit.name,
        price: fruit.price,
        in_stock: fruit.stock > 0
      }
    end
    render json: result
  end
end

🙆‍♂️ GOOD

以下のように、fruits_controller.rb にはデータ取得系のアクションを実装し、検索の責務を持つアクションについては fruits/search_controller.rb へ切り出しました。これにより、各コントローラーの責務が明確になり、単一責務の原則に則った実装になります。

また、ActiveRecordのクエリロジックは models/fruit.rb にスコープとして切り出し、再利用性と保守性を向上させています。さらに、データ整形の責務を専用のシリアライザ fruit_serializer.rb に分離することで、コントローラーが軽量化され、ロジックがシンプルになりました。

app/controllers/api/fruits_controller.rb
class Api::FruitsController < ApplicationController
  def index
    fruits = Fruit.cheap(500) # 例: 500円以下のフルーツ
    render json: fruits.map { |fruit| FruitSerializer.new(fruit).as_json }
  end

  def show
    fruit = Fruit.find(params[:id])
    render json: FruitSerializer.new(fruit).as_json
  end
end
app/controllers/api/fruits/search_controller.rb
class Api::Fruits::SearchController < ApplicationController
  def search
    fruits = Fruit.search_by_name(params[:query])
    render json: fruits.map { |fruit| FruitSerializer.new(fruit).as_json }
  end
end
app/models/fruit.rb
class Fruit < ApplicationRecord
  scope :cheap, ->(max_price) { where('price <= ?', max_price) }
  scope :search_by_name, ->(query) { where('name LIKE ?', "%#{query}%") }
end
app/serializers/fruit_serializer.rb
class FruitSerializer
  def initialize(fruit)
    @fruit = fruit
  end

  def as_json
    {
      id: @fruit.id,
      name: @fruit.name,
      price: @fruit.price,
      in_stock: @fruit.stock > 0
    }
  end
end

私が実装したFat Controller

次に私が実装したFat Controllerのコードを元に問題点とリファクタリング案を考えていきたいと思います。
今回もフルーツECサイトを仮定して説明します。

🙅‍♂️ BAD

app/controllers/api/fruits/orders_controller.rb
class Api::Fruits::OrdersController < Api::BaseController
  before_action :set_order, only: %i[update cancel details]
  before_action :validate_cart, only: %i[create]

  # 現在進行中の注文を取得
  def current_order_summary
    current_order = current_user.orders.where(status: 'in_progress').last

    if current_order.blank?
      render json: { order_status: false }
    else
      order_items = current_order.order_items.includes(:product)
      total_price = order_items.sum { |item| item.product.price * item.quantity }
      shipping_address = current_order.shipping_address

      order_summary = {
        id: current_order.id,
        status: current_order.status,
        shipping_address: shipping_address,
        total_price: total_price,
        items: order_items.map do |item|
          {
            product_name: item.product.name,
            quantity: item.quantity,
            price: item.product.price
          }
        end
      }
      render json: { order_status: true, order_summary: order_summary }
    end
  end

  # 新規の注文を作成
  def create
    new_order = current_user.orders.build(order_params)

    if new_order.save
      cart.items.each do |cart_item|
        new_order.order_items.create(product_id: cart_item.product_id, quantity: cart_item.quantity)
      end
      cart.clear
      render json: { message: 'Order created successfully', order_id: new_order.id }, status: :created
    else
      render json: { message: 'Failed to create order', errors: new_order.errors.full_messages }, status: :unprocessable_entity
    end
  end

  # 注文情報の更新
  def update
    if @order.update(order_params)
      render json: { message: 'Order updated successfully', order: @order }, status: :ok
    else
      render json: { message: 'Failed to update order', errors: @order.errors.full_messages }, status: :unprocessable_entity
    end
  end

  # 注文の詳細情報
  def details
    if @order.present?
      order_details = {
        id: @order.id,
        status: @order.status,
        items: @order.order_items.map do |item|
          {
            product_name: item.product.name,
            quantity: item.quantity,
            price: item.product.price
          }
        end,
        total_price: @order.order_items.sum { |item| item.product.price * item.quantity },
        shipping_address: @order.shipping_address
      }
      render json: { order: order_details }, status: :ok
    else
      render json: { message: 'Order not found' }, status: :not_found
    end
  end

  # 注文のキャンセル
  def cancel
    if @order.cancelable?
      @order.update(status: 'canceled')
      render json: { message: 'Order canceled successfully' }, status: :ok
    else
      render json: { message: 'Order cannot be canceled' }, status: :unprocessable_entity
    end
  end

  # 商品検索
  def search
    keyword = params[:keyword]
    orders = current_user.orders.joins(order_items: :product)
                     .where('products.name ILIKE ?', "%#{keyword}%")
                     .distinct

    if orders.present?
      search_results = orders.map do |order|
        {
          id: order.id,
          status: order.status,
          total_price: order.order_items.sum { |item| item.product.price * item.quantity },
          items: order.order_items.map do |item|
            {
              product_name: item.product.name,
              quantity: item.quantity,
              price: item.product.price
            }
          }
        }
      end
      render json: { results: search_results }, status: :ok
    else
      render json: { message: 'No orders found matching the search criteria' }, status: :not_found
    end
  end

  # 過去の注文取得
  def past_orders
    past_orders = current_user.orders.where(status: 'completed').order(created_at: :desc)

    if past_orders.present?
      past_orders_data = past_orders.map do |order|
        {
          id: order.id,
          status: order.status,
          completed_at: order.updated_at.strftime('%Y-%m-%d %H:%M'),
          total_price: order.order_items.sum { |item| item.product.price * item.quantity },
          items: order.order_items.map do |item|
            {
              product_name: item.product.name,
              quantity: item.quantity,
              price: item.product.price
            }
          }
        }
      end
      render json: { past_orders: past_orders_data }, status: :ok
    else
      render json: { message: 'No past orders found' }, status: :not_found
    end
  end

  private

  def set_order
    @order = current_user.orders.find_by(id: params[:id])
    unless @order
      render json: { message: 'Order not found' }, status: :not_found
    end
  end

  def order_params
    params.require(:order).permit(:shipping_address, :payment_method, order_items_attributes: %i[product_id quantity])
  end

  def validate_cart
    if cart.items.empty?
      render json: { message: 'Your cart is empty' }, status: :unprocessable_entity
    end
  end

  def cart
    @cart ||= current_user.cart
  end
end

いい感じにFat Controllerな実装になっていますね笑
次にFat Controllerの原因となっている箇所について説明します。

Fat Controllerの原因

◼︎ 注文に関する責務が混在したアクションが同じクラスに定義されている。

current_order_summary past_orders searchはデータ取得ロジック・データ加工・レスポンス生成の責務を含んでいます。そのため、単一責務の原則に則っておらず、Fat Controllerの原因になっています。

◼︎ 似ているロジックを抽象化したクラスに切り出せていない。

current_order_summary past_orders detailsで合計金額の計算や商品情報のフォーマットが繰り返されているため、共通ロジックとして抽象化することができる。

◼︎ モデルのスコープに切り出せていない。

current_user.orders.where(status: 'in_progress') や current_user.orders.where(status: 'completed').order(created_at: :desc) のようなメソッドチェーンがアクション内に直接書かれています。

スコープを導入することで、複雑なクエリを簡潔に記述でき、他の箇所でも再利用可能になります。また、コントローラーに直接クエリを書くことを避けられるため、可読性が向上します。

では、次にリファクタリングをしながら、Fat Controllerを解消していきたいと思います。

Fat Controllerを解消

◼︎ 単一責務の原則に則って責務をクラスごとに分割

  • 現在進行中の注文を取得するアクションの current_order_summary

🙆‍♂️ GOOD

app/controllers/api/fruits/current_order_summary.rb
class CurrentOrderSummary
  def initialize(user)
    @user = user
  end

  def call
    current_order = @user.orders.in_progress.last
    return { order_status: false } unless current_order

    {
      order_status: true,
      order_summary: OrderDetailsPresenter.new(current_order).as_json
    }
  end
end

上記のようにクラスを分割することにより、app/controllers/api/fruits/orders_controller.rbでは以下のように呼び出すだけになります。

app/controllers/api/fruits/orders_controller.rb
  # 現在進行中の注文を取得
  def current_order_summary
    render json: CurrentOrderSummary.new(current_user).call
  end

その他のアクションも同様の形でクラスに分割しています。

  • 新規の注文を作成する create

🙆‍♂️ GOOD

app/controllers/api/fruits/order_creator.rb
class OrderCreator
  Result = Struct.new(:success?, :order, :error_message)

  def initialize(user, order_params, cart)
    @user = user
    @order_params = order_params
    @cart = cart
  end

  def call
    order = @user.orders.build(@order_params)
    if order.save
      @cart.items.each do |cart_item|
        order.order_items.create(product_id: cart_item.product_id, quantity: cart_item.quantity)
      end
      @cart.clear
      Result.new(true, order, nil)
    else
      Result.new(false, nil, order.errors.full_messages.to_sentence)
    end
  end
end
app/controllers/api/fruits/orders_controller.rb
  # 新規の注文を作成
  def create
    result = OrderCreator.new(current_user, order_params, cart).call
    if result.success?
      render json: { message: 'Order created successfully', order_id: result.order.id }, status: :created
    else
      render json: { message: result.error_message }, status: :unprocessable_entity
    end
  end
  • 注文情報を更新する update

🙆‍♂️ GOOD

app/controllers/api/fruits/order_updater.rb
class OrderUpdater
  Result = Struct.new(:success?, :error_message)

  def initialize(order, params)
    @order = order
    @params = params
  end

  def call
    if @order.update(@params)
      Result.new(true, nil)
    else
      Result.new(false, @order.errors.full_messages.to_sentence)
    end
  end
end
app/controllers/api/fruits/orders_controller.rb
  # 注文情報の更新
  def update
    result = OrderUpdater.new(@order, order_params).call
    if result.success?
      render json: { message: 'Order updated successfully', order: @order }, status: :ok
    else
      render json: { message: result.error_message }, status: :unprocessable_entity
    end
  end
  • 注文の詳細情報する details

OrderDetailsPresenterクラスでは、共通のロジックを抽出してまとめています。
これにより再利用することが可能になりました。

🙆‍♂️ GOOD

app/controllers/api/fruits/order_details_presenter.rb
class OrderDetailsPresenter
  def initialize(order)
    @order = order
  end

  def as_json
    {
      id: @order.id,
      status: @order.status,
      total_price: @order.order_items.sum { |item| item.product.price * item.quantity },
      shipping_address: @order.shipping_address,
      items: @order.order_items.map do |item|
        {
          product_name: item.product.name,
          quantity: item.quantity,
          price: item.product.price
        }
      end
    }
  end
end
app/controllers/api/fruits/orders_controller.rb
  # 注文の詳細情報
  def details
    render json: OrderDetailsPresenter.new(@order).as_json
  end
  • 注文のキャンセルする cancel
app/controllers/api/fruits/order_canceler.rb
class OrderCanceler
  Result = Struct.new(:success?, :error_message)

  def initialize(order)
    @order = order
  end

  def call
    if @order.cancelable?
      @order.update(status: 'canceled')
      Result.new(true, nil)
    else
      Result.new(false, 'Order cannot be canceled')
    end
  end
end
app/controllers/api/fruits/orders_controller.rb
  # 注文のキャンセル
  def cancel
    result = OrderCanceler.new(@order).call
    if result.success?
      render json: { message: 'Order canceled successfully' }, status: :ok
    else
      render json: { message: result.error_message }, status: :unprocessable_entity
    end
  end
  • 商品を検索する search

🙆‍♂️ GOOD

app/controllers/api/fruits/order_searcher.rb
class OrderSearcher
  def initialize(user, keyword)
    @user = user
    @keyword = keyword
  end

  def call
    orders = @user.orders.joins(order_items: :product)
                 .where('products.name ILIKE ?', "%#{@keyword}%")
                 .distinct

    orders.map { |order| OrderDetailsPresenter.new(order).as_json }
  end
end
app/controllers/api/fruits/orders_controller.rb
  # 商品検索
  def search
    results = OrderSearcher.new(current_user, params[:keyword]).call
    if results.present?
      render json: { results: results }, status: :ok
    else
      render json: { message: 'No orders found matching the search criteria' }, status: :not_found
    end
  end
  • 過去の注文取得する past_orders

🙆‍♂️ GOOD

app/controllers/api/fruits/order_past_order.rb
class PastOrdersFetcher
  def initialize(user)
    @user = user
  end

  def call
    past_orders = @user.orders.completed
    past_orders.map { |order| OrderDetailsPresenter.new(order).as_json }
  end
end
app/controllers/api/fruits/orders_controller.rb
  # 過去の注文取得
  def past_orders
    render json: PastOrdersFetcher.new(current_user).call
  end

◼︎ モデルにスコープの導入

🙆‍♂️ GOOD

app/models/order.rb
class Order < ApplicationRecord
  scope :in_progress, -> { where(status: 'in_progress') }
  scope :completed, -> { where(status: 'completed').order(created_at: :desc) }
end
app/controllers/api/fruits/orders_controller.rbの全体コード
class Api::Fruits::OrdersController < Api::BaseController
  before_action :set_order, only: %i[update cancel details]
  before_action :validate_cart, only: %i[create]

  # 現在進行中の注文を取得
  def current_order_summary
    render json: CurrentOrderSummary.new(current_user).call
  end

  # 新規の注文を作成
  def create
    result = OrderCreator.new(current_user, order_params, cart).call
    if result.success?
      render json: { message: 'Order created successfully', order_id: result.order.id }, status: :created
    else
      render json: { message: result.error_message }, status: :unprocessable_entity
    end
  end

  # 注文情報の更新
  def update
    result = OrderUpdater.new(@order, order_params).call
    if result.success?
      render json: { message: 'Order updated successfully', order: @order }, status: :ok
    else
      render json: { message: result.error_message }, status: :unprocessable_entity
    end
  end

  # 注文の詳細情報
  def details
    render json: OrderDetailsPresenter.new(@order).as_json
  end

  # 注文のキャンセル
  def cancel
    result = OrderCanceler.new(@order).call
    if result.success?
      render json: { message: 'Order canceled successfully' }, status: :ok
    else
      render json: { message: result.error_message }, status: :unprocessable_entity
    end
  end

  # 商品検索
  def search
    results = OrderSearcher.new(current_user, params[:keyword]).call
    if results.present?
      render json: { results: results }, status: :ok
    else
      render json: { message: 'No orders found matching the search criteria' }, status: :not_found
    end
  end

  # 過去の注文取得
  def past_orders
    render json: PastOrdersFetcher.new(current_user).call
  end

  private

  def set_order
    @order = current_user.orders.find_by(id: params[:id])
    render json: { message: 'Order not found' }, status: :not_found unless @order
  end

  def order_params
    params.require(:order).permit(:shipping_address, :payment_method, order_items_attributes: %i[product_id quantity])
  end

  def validate_cart
    render json: { message: 'Your cart is empty' }, status: :unprocessable_entity if cart.items.empty?
  end

  def cart
    @cart ||= current_user.cart
  end
end

このように「単一責務のクラス」「共通ロジックの抽出」「モデルのスコープを導入」を軸にリファクタリングを行うとFat Controllerが解消されたのではないでしょうか？
また、クラス・ロジックの再利用性・保守性も向上したかと思います。

まとめ

今回は、Fat Controllerの問題点を整理し、その原因と改善方法を具体的に見てきました。
最後に要点をまとめて終わりにしようと思います。

◼︎ Fat Controllerの原因

コントローラーに複数の責務が混在していたこと。
共通ロジックが重複し、コードの再利用性が低かったこと。
ActiveRecordのクエリが直接記述され、複雑性を増していたこと。

◼︎ 改善方法

単一責務の原則に則り、各アクションを専用クラスに切り出すことで、責務を明確化。
共通ロジックを抽出して再利用可能なクラス（プレゼンターやヘルパー）にまとめた。
ActiveRecordのスコープを導入し、メソッドチェーンを簡潔に記述。

◼︎ 改善後のメリット

コントローラーが軽量化され、役割が明確になり、コードの可読性が向上。
再利用性が高まり、ロジックの修正が他の箇所に影響を与えにくくなった。
クラスやメソッドが独立してテストしやすくなり、品質の向上にもつながった。

リファクタリングを通じて、単一責務の原則やコードの抽象化、再利用性の重要性を改めて実感しました。
今後も、これらの原則を念頭に置きながら、読みやすく変更に強いコードを書いていきたいと思います。

参考情報

https://techracho.bpsinc.jp/hachi8833/2021_10_05/112108
https://techracho.bpsinc.jp/hachi8833/2018_03_27/54130

GitHubで編集を提案

Discussion