mybook

Refactorability 実践 — Railsリファクタリングカタログ

リファクタリングとは何か

「コードの振る舞いを変えずに、内部構造を改善する作業」

これがリファクタリングの定義だ。ユイは田中さんとペアプログラミングをしながら、実際のコードでリファクタリングを体験した。

「なぜ振る舞いを変えないのが重要なんですか?」ユイが聞いた。

「リファクタリングとバグ修正を同時にやると、どちらのせいでテストが壊れたかわからなくなる。リファクタリングは『構造の改善』だけに集中する。バグを見つけたら別のコミットで直す」

WARNING

リファクタリングの前に必ずテストを書いてください。テストなしでリファクタリングすることは、安全ネットなしで空中ブランコをするようなものです。「テストが通っていること」がリファクタリング前後で変化がないことの唯一の証明です。

リファクタリング前の準備

# ステップ1: テストがグリーンであることを確認
bundle exec rspec --format progress
# ...............................
# 31 examples, 0 failures  ← グリーンを確認
 
# ステップ2: カバレッジを確認(低いなら先にテストを追加)
COVERAGE=true bundle exec rspec
# Coverage report generated at coverage/index.html
# LOC: 1,234, LOC tested: 987, Coverage: 79.98%
 
# ステップ3: リファクタリング対象をブランチに切る
git checkout -b refactoring/order-service-decomposition
 
# ステップ4: 小さな変更→テスト→コミットのサイクルを回す
# 1回の変更で何十箇所も変えない

カタログ1: Extract Method(メソッドの抽出)

最も基本的で最も使うリファクタリング。「コードの塊に名前をつける」作業だ。

# Before: 65行のメソッド(複数の処理が混在)
def create_order(user, cart, coupon_code: nil)
  order = Order.new(user: user)
 
  # === カート内アイテムをコピー ===
  cart.items.each do |cart_item|
    order.line_items.build(
      product: cart_item.product,
      quantity: cart_item.quantity,
      unit_price: cart_item.product.current_price
    )
  end
 
  # === 合計金額を計算 ===
  subtotal = order.line_items.sum { |li| li.unit_price * li.quantity }
  tax_rate = 0.10
  tax = subtotal * tax_rate
  order.subtotal = subtotal
  order.tax = tax.round(2)
  order.total = order.subtotal + order.tax
 
  # === プレミアム割引 ===
  if user.premium?
    discount = order.total * 0.20
    order.discount = discount.round(2)
    order.total -= order.discount
  end
 
  # === クーポン適用 ===
  if coupon_code.present?
    coupon = Coupon.find_by(code: coupon_code)
    if coupon && !coupon.expired? && !coupon.used? && coupon.minimum_order <= order.total
      order.total -= coupon.discount_amount
      order.coupon = coupon
    end
  end
 
  # === 在庫確認 ===
  order.line_items.each do |line_item|
    if line_item.product.stock < line_item.quantity
      raise InsufficientStockError, "#{line_item.product.name}の在庫が不足しています"
    end
  end
 
  order.save!
  order
end
# After: Extract Methodで各処理を名前のついたメソッドに抽出
def create_order(user, cart, coupon_code: nil)
  order = Order.new(user: user)
 
  copy_cart_items(order, cart)
  calculate_totals(order)
  apply_premium_discount(order, user)
  apply_coupon(order, coupon_code)
  validate_stock_availability(order)
 
  order.save!
  order
end
 
private
 
def copy_cart_items(order, cart)
  cart.items.each do |cart_item|
    order.line_items.build(
      product: cart_item.product,
      quantity: cart_item.quantity,
      unit_price: cart_item.product.current_price
    )
  end
end
 
def calculate_totals(order)
  subtotal = order.line_items.sum { |li| li.unit_price * li.quantity }
  order.subtotal = subtotal
  order.tax = (subtotal * TAX_RATE).round(2)
  order.total = order.subtotal + order.tax
end
 
def apply_premium_discount(order, user)
  return unless user.premium?
 
  discount = (order.total * PREMIUM_DISCOUNT_RATE).round(2)
  order.discount = discount
  order.total -= discount
end
 
def apply_coupon(order, coupon_code)
  return if coupon_code.blank?
 
  coupon = Coupon.find_by(code: coupon_code)
  return unless coupon&.applicable_to?(order)
 
  order.total -= coupon.discount_amount
  order.coupon = coupon
end
 
def validate_stock_availability(order)
  order.line_items.each do |line_item|
    next if line_item.product.stock >= line_item.quantity
 
    raise InsufficientStockError,
          "#{line_item.product.name}の在庫が不足しています(必要: #{line_item.quantity}, 在庫: #{line_item.product.stock})"
  end
end

改善の効果:

  • create_order メソッドが10行に収まり、「何をするか」が一目でわかる
  • 各サブメソッドが独立してテスト可能になる
  • apply_coupon のロジックを変えても calculate_totals には影響しない

カタログ2: Replace Conditional with Polymorphism(条件をポリモーフィズムに置き換える)

case 文や if/elsif の連鎖が増え続けるとき、ポリモーフィズム(多態性)で解決する。

# Before: 配送タイプの条件分岐があちこちに散在する
def calculate_shipping_fee(order)
  case order.shipping_type
  when 'standard'
    order.total >= 5_000 ? 0 : 500
  when 'express'
    order.total >= 10_000 ? 0 : 1_500
  when 'same_day'
    2_500
  when 'international'
    order.total >= 50_000 ? 3_000 : 5_000
  end
end
 
def estimated_delivery_message(order)
  case order.shipping_type
  when 'standard' then "3〜5営業日でお届けします"
  when 'express' then "翌営業日にお届けします"
  when 'same_day' then "本日中にお届けします"
  when 'international' then "7〜14営業日でお届けします"
  end
end
 
def tracking_available?(order)
  case order.shipping_type
  when 'standard', 'express' then true
  when 'same_day' then false
  when 'international' then order.total >= 10_000
  end
end
# → 新しい配送タイプが増えると、全部のcase文に追加が必要
# After: 配送タイプをクラスで表現する
module Shipping
  class Standard
    FREE_THRESHOLD = 5_000
    BASE_FEE = 500
 
    def fee(order)
      order.total >= FREE_THRESHOLD ? 0 : BASE_FEE
    end
 
    def delivery_message
      "3〜5営業日でお届けします"
    end
 
    def tracking_available?(_order)
      true
    end
 
    def label
      '通常配送'
    end
  end
 
  class Express
    FREE_THRESHOLD = 10_000
    BASE_FEE = 1_500
 
    def fee(order)
      order.total >= FREE_THRESHOLD ? 0 : BASE_FEE
    end
 
    def delivery_message
      "翌営業日にお届けします"
    end
 
    def tracking_available?(_order)
      true
    end
 
    def label
      '速達配送'
    end
  end
 
  class SameDay
    FLAT_FEE = 2_500
 
    def fee(_order)
      FLAT_FEE
    end
 
    def delivery_message
      "本日中にお届けします"
    end
 
    def tracking_available?(_order)
      false  # 当日配送はリアルタイム追跡なし
    end
 
    def label
      '当日配送'
    end
  end
 
  class International
    FREE_THRESHOLD = 50_000
    STANDARD_FEE = 5_000
    DISCOUNTED_FEE = 3_000
    TRACKING_THRESHOLD = 10_000
 
    def fee(order)
      order.total >= FREE_THRESHOLD ? 0 :
        order.total >= TRACKING_THRESHOLD ? DISCOUNTED_FEE : STANDARD_FEE
    end
 
    def delivery_message
      "7〜14営業日でお届けします(税関通過に時間がかかる場合があります)"
    end
 
    def tracking_available?(order)
      order.total >= TRACKING_THRESHOLD
    end
 
    def label
      '国際配送'
    end
  end
end
 
class Order < ApplicationRecord
  SHIPPING_STRATEGIES = {
    'standard' => Shipping::Standard,
    'express' => Shipping::Express,
    'same_day' => Shipping::SameDay,
    'international' => Shipping::International
  }.freeze
 
  def shipping_strategy
    strategy_class = SHIPPING_STRATEGIES.fetch(shipping_type) do
      raise UnknownShippingType, "未知の配送タイプ: #{shipping_type}"
    end
    strategy_class.new
  end
 
  def shipping_fee
    shipping_strategy.fee(self)
  end
 
  def delivery_message
    shipping_strategy.delivery_message
  end
 
  def tracking_available?
    shipping_strategy.tracking_available?(self)
  end
end

新しい「ドローン配送」を追加するとき:

  1. Shipping::Drone クラスを作成する
  2. SHIPPING_STRATEGIES に登録する

既存のStandard、Express、SameDay、Internationalのコードには一切触れない。

Loading diagram...

カタログ3: Introduce Parameter Object(パラメータオブジェクトの導入)

引数が多すぎると呼び出しが難しくなり、順番を間違えやすい。

# Before: 8個の引数(呼び出しのたびに順番を確認しなければならない)
def search_orders(user_id, start_date, end_date, status, min_total, max_total, page, per_page)
  Order.where(user_id: user_id)
       .where(status: status)
       .where(created_at: start_date..end_date)
       .where(total: min_total..max_total)
       .page(page).per(per_page)
end
 
# 呼び出し側: 引数の順番を間違えやすい(min_totalとmax_totalを逆にしてしまうなど)
search_orders(user.id, 1.month.ago, Time.now, 'completed', 1000, 50_000, 1, 20)
#             ↑         ↑            ↑         ↑            ↑     ↑         ↑  ↑
# 8つの引数を正しい順で覚える必要がある
# After: パラメータオブジェクト(Value Object)を使う
class OrderSearchParams
  attr_reader :user_id, :status, :page, :per_page
 
  def initialize(params = {})
    @user_id    = params[:user_id]
    @status     = params[:status]
    @start_date = params[:start_date]&.beginning_of_day
    @end_date   = params[:end_date]&.end_of_day
    @min_total  = params[:min_total]
    @max_total  = params[:max_total]
    @page       = params.fetch(:page, 1).to_i
    @per_page   = params.fetch(:per_page, 20).to_i
  end
 
  def date_range
    @start_date..@end_date
  end
 
  def total_range
    @min_total..@max_total
  end
 
  def valid?
    user_id.present? &&
      (start_date.nil? || end_date.nil? || @start_date <= @end_date)
  end
 
  def validation_errors
    errors = []
    errors << 'ユーザーIDが必要です' unless user_id.present?
    errors << '開始日が終了日より後です' if @start_date && @end_date && @start_date > @end_date
    errors
  end
end
 
def search_orders(search_params)
  return Order.none unless search_params.valid?
 
  scope = Order.where(user_id: search_params.user_id)
  scope = scope.where(status: search_params.status) if search_params.status.present?
  scope = scope.where(created_at: search_params.date_range) if search_params.date_range.any?
  scope = scope.where(total: search_params.total_range) if search_params.min_total || search_params.max_total
  scope.page(search_params.page).per(search_params.per_page)
end
 
# 呼び出し側: 名前付きで意図が明確
params = OrderSearchParams.new(
  user_id: user.id,
  start_date: 1.month.ago,
  end_date: Time.current,
  status: 'completed',
  min_total: 1_000,
  max_total: 50_000
)
search_orders(params)

パラメータオブジェクトのメリット:

  • 引数の順番を覚える必要がなくなる(名前付きハッシュ)
  • バリデーションロジックをオブジェクトに閉じ込められる
  • 将来パラメータが増えても、呼び出し側の変更が最小限
  • テストが書きやすい(パラメータオブジェクト単体でテスト可能)

カタログ4: Replace Temp with Query(一時変数をクエリメソッドに置き換える)

# Before: 一時変数が処理の流れを追いにくくする
def generate_invoice(order)
  items = order.line_items
  subtotal = items.sum { |i| i.unit_price * i.quantity }
  tax = subtotal * 0.10
  discount = order.coupon ? order.coupon.discount_amount : 0
  total = subtotal + tax - discount
  order_count = order.user.orders.completed.count
  is_first_order = order_count == 1
  loyalty_points = (total * 0.01).floor
 
  {
    subtotal: subtotal,
    tax: tax,
    discount: discount,
    total: total,
    loyalty_points_earned: loyalty_points,
    is_first_order: is_first_order
  }
end
# After: Value Objectとクエリメソッドで整理する
class InvoiceSummary
  TAX_RATE = 0.10
  LOYALTY_POINTS_RATE = 0.01
 
  def initialize(order)
    @order = order
  end
 
  def subtotal
    @subtotal ||= @order.line_items.sum { |i| i.unit_price * i.quantity }
  end
 
  def tax
    (subtotal * TAX_RATE).round(2)
  end
 
  def discount
    @order.coupon&.discount_amount || 0
  end
 
  def total
    (subtotal + tax - discount).round(2)
  end
 
  def loyalty_points_earned
    (total * LOYALTY_POINTS_RATE).floor
  end
 
  def first_order?
    @order.user.orders.completed.count == 1
  end
 
  def to_h
    {
      subtotal: subtotal,
      tax: tax,
      discount: discount,
      total: total,
      loyalty_points_earned: loyalty_points_earned,
      first_order: first_order?
    }
  end
end
 
# 使用例
summary = InvoiceSummary.new(order)
puts summary.total
puts summary.loyalty_points_earned
render json: summary.to_h

改善の効果:

  • 各計算が独立してテスト可能
  • ||= による遅延評価で同じ計算が2回走らない
  • InvoiceSummary オブジェクトをメソッド間で渡せる

カタログ5: Extract Class(クラスの抽出)

モデルが「2つのことをしている」と感じたら、クラスを抽出する。

# Before: Productモデルが価格計算と在庫管理の両方を担っている
class Product < ApplicationRecord
  # 価格関連(200行)
  def current_price
    base_price - active_discount_amount
  end
 
  def active_discount_amount
    discounts.active.sum(:amount)
  end
 
  def price_for_tier(user_tier)
    base_price * (1 - TIER_DISCOUNTS[user_tier])
  end
 
  def price_history
    price_changes.order(:created_at)
  end
 
  # 在庫関連(200行)
  def in_stock?
    stock > 0
  end
 
  def low_stock?
    stock < reorder_point
  end
 
  def reserve_stock(quantity)
    update!(reserved_stock: reserved_stock + quantity)
  end
 
  def release_reservation(quantity)
    update!(reserved_stock: [reserved_stock - quantity, 0].max)
  end
 
  def available_stock
    stock - reserved_stock
  end
end
 
# After: 責任を別クラスに切り出す
class Product < ApplicationRecord
  # Productはコアデータのみ
  has_many :price_changes
  has_many :stock_movements
end
 
class ProductPricing
  TIER_DISCOUNTS = {
    premium: 0.10,
    gold: 0.05,
    standard: 0.00
  }.freeze
 
  def initialize(product)
    @product = product
  end
 
  def current_price
    @product.base_price - active_discount_amount
  end
 
  def price_for_tier(user_tier)
    current_price * (1 - TIER_DISCOUNTS.fetch(user_tier, 0))
  end
 
  def active_discount_amount
    @product.discounts.active.sum(:amount)
  end
 
  def price_history
    @product.price_changes.order(:created_at)
  end
end
 
class ProductInventory
  LOW_STOCK_THRESHOLD = 5
 
  def initialize(product)
    @product = product
  end
 
  def in_stock?
    available > 0
  end
 
  def low_stock?
    available < LOW_STOCK_THRESHOLD
  end
 
  def available
    @product.stock - @product.reserved_stock
  end
 
  def reserve!(quantity)
    raise InsufficientStockError if available < quantity
    @product.increment!(:reserved_stock, quantity)
  end
 
  def release_reservation!(quantity)
    @product.update!(reserved_stock: [@product.reserved_stock - quantity, 0].max)
  end
end
 
# 使用例
product = Product.find(1)
pricing = ProductPricing.new(product)
inventory = ProductInventory.new(product)
 
puts pricing.current_price
puts pricing.price_for_tier(:premium)
inventory.reserve!(3) if inventory.in_stock?

INFO

リファクタリングカタログ(Martin Fowler著「Refactoring」)には100種以上のパターンが載っています。しかし実務で最も使うのは: Extract MethodReplace Conditional with PolymorphismIntroduce Parameter Object の3つです。この3つをマスターするだけで、コードの品質は大幅に改善します。

リファクタリングのワークフロー: Red-Green-Refactor

テスト駆動開発(TDD)のサイクルに乗せてリファクタリングする。

# ステップ1: Red(テストを書いて失敗させる)
# ※ リファクタリングの場合は「現状の振る舞いを記述するテスト」を書く
RSpec.describe 'OrderCreationService(リファクタリング前)' do
  it '注文を作成する' do
    user = create(:user, :premium)
    cart = create(:cart, :with_items, total: 10_000)
 
    order = create_order(user, cart)  # ← 現状のメソッドを呼ぶ
 
    expect(order).to be_persisted
    expect(order.total).to eq(8_000)  # プレミアム割引後
  end
end
 
# ステップ2: Green(テストを通す確認)
bundle exec rspec spec/services/order_creation_service_spec.rb
# 1 example, 0 failures  ← グリーンを確認
 
# ステップ3: Refactor(振る舞いを変えずに内部を改善)
# Extract Methodを適用
def create_order(user, cart, coupon_code: nil)
  order = Order.new(user: user)
  copy_cart_items(order, cart)         # 抽出
  calculate_totals(order)              # 抽出
  apply_premium_discount(order, user)  # 抽出
  apply_coupon(order, coupon_code)     # 抽出
  order.save!
  order
end
 
# ステップ4: Green(リファクタリング後もテストが通るか確認)
bundle exec rspec spec/services/order_creation_service_spec.rb
# 1 example, 0 failures  ← グリーン(振る舞いが変わっていない)
 
# ステップ5: コミット(小さな単位でコミットする)
git add app/services/order_creation_service.rb
git commit -m "refactor: Extract Method for order creation steps"

ユイのペアプログラミング体験記

「リファクタリングは怖くなかった」ユイは日報に書いた。

「テストがあったから、変えながら確認できた。あの最初のプロローグのコードを見たときの恐怖——あれはテストがなかったから。1行変えるたびに『これで合ってるのか?』と不安だった。でも今日は違う。テストがグリーンのまま内部構造を改善できた。テストこそが変更を可能にする安全網だ」

リファクタリングパターン解決する問題適用タイミング
Extract Method長すぎるメソッドメソッドが15行を超えたとき
Replace Conditional with Polymorphism増え続けるcase/if新しい種類が追加されるたびにcase文を変えているとき
Introduce Parameter Object引数が多すぎる引数が4個以上になったとき
Replace Temp with Query一時変数が多い変数を追うのが困難になったとき
Extract Class1クラスが大きくなりすぎる1ファイルが200行を超えたとき

実践課題: ケンタのコードをリファクタリングする

「ユイさん、ちょっと見てもらえますか」

翌週、後輩のケンタが声をかけてきた。コードレビューでもらったコメントを修正しようとしているが、どこから手をつければいいかわからないという。

# ケンタの最初のコード(PR前)
def generate_monthly_report(month, year)
  start_date = Date.new(year, month, 1)
  end_date = Date.new(year, month, -1)
  orders = Order.where(created_at: start_date.beginning_of_day..end_date.end_of_day).completed
  total = 0
  orders.each { |o| total += o.total }
  avg = orders.count > 0 ? total / orders.count : 0
  top_products = {}
  orders.each do |o|
    o.line_items.each do |li|
      top_products[li.product.name] ||= 0
      top_products[li.product.name] += li.quantity
    end
  end
  top_5 = top_products.sort_by { |k, v| -v }.first(5)
  { total: total, average: avg, top_products: top_5, order_count: orders.count, month: "#{year}/#{month}" }
end

「まずテストを書こう」ユイが言った。「振る舞いを固定してから、安心してリファクタリングできる」

# Step 1: テストを書いてグリーンにする
RSpec.describe 'generate_monthly_report' do
  it '月次レポートを生成する' do
    create(:order, :completed, total: 10_000, created_at: Date.new(2024, 3, 15))
    create(:order, :completed, total: 20_000, created_at: Date.new(2024, 3, 20))
 
    report = generate_monthly_report(3, 2024)
 
    expect(report[:order_count]).to eq(2)
    expect(report[:total]).to eq(30_000)
    expect(report[:average]).to eq(15_000)
    expect(report[:month]).to eq('2024/3')
  end
end
# Step 2: Extract Method でリファクタリング
def generate_monthly_report(month, year)
  period = MonthlyPeriod.new(year, month)
  orders = completed_orders_in(period)
 
  {
    month: period.label,
    order_count: orders.count,
    total: calculate_total(orders),
    average: calculate_average(orders),
    top_products: top_products_for(orders)
  }
end
 
private
 
def completed_orders_in(period)
  Order.completed.where(created_at: period.date_range)
end
 
def calculate_total(orders)
  orders.sum(:total)
end
 
def calculate_average(orders)
  return 0 if orders.empty?
  (calculate_total(orders) / orders.count).round
end
 
def top_products_for(orders, limit: 5)
  product_quantities = Hash.new(0)
  orders.flat_map(&:line_items).each do |line_item|
    product_quantities[line_item.product.name] += line_item.quantity
  end
  product_quantities.max_by(limit) { |_name, qty| qty }
end
 
# Value Object: 月の期間を表す
class MonthlyPeriod
  def initialize(year, month)
    @year = year
    @month = month
  end
 
  def start_date
    Date.new(@year, @month, 1)
  end
 
  def end_date
    Date.new(@year, @month, -1)  # -1 = 末日
  end
 
  def date_range
    start_date.beginning_of_day..end_date.end_of_day
  end
 
  def label
    "#{@year}/#{@month}"
  end
end

「テストを実行してグリーンを確認」

bundle exec rspec spec/reports/monthly_report_spec.rb
# 1 example, 0 failures

「完璧です」ケンタが言った。「リファクタリング前後でテストが通ってる。これが安全ネットだったんですね」

「次は、チームでこれらを実践するコードレビューの話をしよう」田中さんが締めた。「3Rの知識は1人で持っていても限界がある。チームが共通の言語で設計について話せるようになると、コードの品質がチームとして上がっていく」