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新しい「ドローン配送」を追加するとき:
Shipping::Droneクラスを作成するSHIPPING_STRATEGIESに登録する
既存のStandard、Express、SameDay、Internationalのコードには一切触れない。
カタログ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 Method、Replace Conditional with Polymorphism、Introduce 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 Class | 1クラスが大きくなりすぎる | 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人で持っていても限界がある。チームが共通の言語で設計について話せるようになると、コードの品質がチームとして上がっていく」