コードの整理と改善

最終更新日： 2024年5月31日（金）
公開日： 2024年5月31日（金）

はじめに

弊社では主力サービスである「ミールタイム」や「旬すぐ」の開発にRuby on Railsを使用しています。Railsは非常に生産性の高いフレームワークですが、プロジェクトが大規模化するにつれて、コードの構造や設計に問題が生じることがあります。

今回は、以下に対してどのようにアプローチしているかを紹介します。

Fat Controller
責務の分離
Viewのロジック

Fat Controller問題への対処

Railsでは「Skinny Controller, Fat Model」の原則に従い、ビジネスロジックはなるべくモデルに寄せ、コントローラはリクエストの受け取りとレスポンスの返却に専念することが推奨されています。
弊社では、コントローラに書かれていた処理を、モデルやサービスクラス、モジュールとして切り出すようにしています。コントローラ内では、主にこれらのモデルやモジュール、サービスクラスを呼び出す処理とレスポンスの記述を行います。

# 変更前
# app/controllers/orders_controller.rb
class OrdersController < ApplicationController
  def create
    @order = Order.new(order_params)
    @order.user = current_user
    @order.total_price = calculate_total_price(@order)
    
    if @order.save
      @order.line_items.each do |line_item|
        product = line_item.product
        product.decrement!(:stock_quantity, line_item.quantity)
      end
      
      OrderMailer.confirmation_email(@order).deliver_now
      redirect_to @order, notice: 'Order was successfully created.'
    else
      render :new
    end
  end

  private

  def order_params
    params.require(:order).permit(:shipping_address, line_items_attributes: [:product_id, :quantity])
  end

  def calculate_total_price(order)
    order.line_items.sum { |line_item| line_item.product.price * line_item.quantity }
  end
end

# 変更後 
# app/controllers/orders_controller.rb
class OrdersController < ApplicationController
  def create
    @order = current_user.orders.build(order_params)
    
    if @order.save
      redirect_to @order, notice: 'Order was successfully created.'
    else
      render :new
    end
  end

  private

  def order_params
    params.require(:order).permit(:shipping_address, line_items_attributes: [:product_id, :quantity])
  end
end

# app/models/order.rb
class Order < ActiveRecord::Base
  belongs_to :user
  has_many :line_items
  accepts_nested_attributes_for :line_items

  before_save :calculate_total_price
  after_create :update_product_stock
  after_create :send_confirmation_email

  private

  def calculate_total_price
    self.total_price = line_items.sum { |line_item| line_item.product.price * line_item.quantity }
  end

  def update_product_stock
    line_items.each do |line_item|
      product = line_item.product
      product.decrement!(:stock_quantity, line_item.quantity)
    end
  end

  def send_confirmation_email
    OrderMailer.confirmation_email(self).deliver_now
  end
end

上記の例では、コントローラからモデルへ処理を移動し、コントローラをスリム化しています。calculate_total_price、update_product_stock、send_confirmation_emailといった処理をモデルのコールバックとして定義することで、コントローラの責務を明確にしています。（実際の現場では、before_saveやafter_createなどのコールバックはあまり使用していません）

責務の分離

現在の本番コードはモデルのインスタンスメソッドやクラスメソッドが肥大化し過ぎており、別モデルの操作まで行っているケースも存在しています。これらに対し、適切な分割と単一責任の原則に従うようにリファクタリングを行っています。また、複数のモデルにまたがる処理は一度サービスクラスに切り分けるようにしています。サービスクラスを作成することで、影響範囲や業務手続きがわかりやすくなります。その後、必要であれば適した粒度に再度リファクタリングをしています。

# 変更前
class User < ActiveRecord::Base
  def self.import_csv(file)
    CSV.foreach(file.path, headers: true) do |row|
      user = find_by(email: row["email"]) || new
      user.attributes = row.to_hash.slice(*updatable_attributes)
      user.save!
    end
  end
end

# 変更後
module Concerns::Service
  extend ActiveSupport::Concern
end

class User::CsvImportService
  include Concerns::Service

  def initialize(file: nil)
    @file = file
  end

  def call
    CSV.foreach(file.path, headers: true) do |row|
      user = User.find_by(email: row["email"]) || User.new
      user.attributes = row.to_hash.slice(*User.updatable_attributes)
      user.save!
    end
  end
end

上記の例では、Userモデルからimport_csvメソッドを切り出し、User::CsvImportServiceというサービスクラスを作成しています。これにより、Userモデルの責務を明確にし、CSVインポートの処理を独立したクラスで管理できるようになります。

Viewのロジック

Viewにロジックが書かれている場合は、ヘルパーメソッドを使用するようにリファクタリングしています。また、フォームに関しては極力フォームオブジェクトパターンを使用するように変更しています。

<!-- 変更前 -->
<% if current_user.admin? || current_user == @post.user %>
  <%= link_to '編集', edit_post_path(@post) %>
  <%= link_to '削除', post_path(@post), method: :delete, data: { confirm: '本当に削除しますか？' } %>
<% end %>

<!-- 変更後 -->
<% if policy(@post).edit? %>
  <%= link_to '編集', edit_post_path(@post) %>
<% end %>
<% if policy(@post).destroy? %>  
  <%= link_to '削除', post_path(@post), method: :delete, data: { confirm: '本当に削除しますか？' } %>
<% end %>

上記の例では、Viewの条件分岐をポリシークラスを使用して置き換えています。これにより、権限チェックのロジックをViewから分離し、コードの可読性と保守性を向上させています。

まとめ

弊社では上記のようなアプローチでRailsアプリケーションのコード品質改善に取り組んでいます。もちろん、これらは一朝一夕にできるものではありません。リファクタリングは段階的に行い、テストを書きながら進めていくことが重要だと考えています。今後もサービスの成長に合わせて、継続的にコードの改善を行っていきたいと思います。

募集要項およびエントリーフォーム