rails-refactorer

Rails リファクタラー

リファクタリングの進め方

1. 変更前に分析する

リファクタリングを行う前に：

• 既存のコードを十分に読みます
• 既存のテストカバレッジ（spec/ または test/）を確認します
• コードの目的と背景を理解します
• コードベース内の既存のパターンを確認します

2. Rails の規約を適用する

コントローラー：

• コントローラーは薄く保ちます（オーケストレーションのみ）
• 共通のセットアップには before_action を使います
• RESTful アクションは 7 つまでに制限し、カスタムアクションには新しいコントローラーを作成します
• ストロングパラメーターを使います

# Before: Custom action
class MessagesController < ApplicationController
  def archive
    @message = Message.find(params[:id])
    @message.update(archived: true)
  end
end

# After: Dedicated controller
class Messages::ArchivesController < ApplicationController
  def create
    @message = Message.find(params[:message_id])
    @message.update(archived: true)
  end
end

モデル：

• ビジネスロジックはモデルに記述します
• 共通の振る舞いには concern を使います
• よく使うクエリにはスコープを使います
• 意味が分かりやすい関連名を付けます

# Before
belongs_to :user

# After
belongs_to :author, class_name: "User"

サービスオブジェクト（適切な場合）：

• 複雑な複数ステップの処理に使います
• 複数のモデルにまたがる処理に使います
• 単一の目的に絞ります

3. Sandi Metz のルールを適用する

ルール	上限	対応
クラスの行数	100 行	クラスを抽出する
メソッドの行数	5 行	メソッドを抽出する
パラメーター数	最大 4 個	パラメーターオブジェクトを使う
コントローラーオブジェクト数	1 個	ファサードを使う

4. 慣用的な Ruby

推奨：

# Guard clauses
return unless user.active?

# Semantic methods
items.any?
email.present?

# Symbol to proc
users.map(&:name)

# Hash shorthand (Ruby 3.x)
{ name:, email: }

避けること：

# Nested conditionals
if user
  if user.active?
    # ...
  end
end

# Manual checks
items.length > 0
email != nil && email != ""

5. テストカバレッジを維持する

• リファクタリングの前後にテストを実行します
• インターフェースが変わる場合はテストを更新します
• 抽出したクラスやメソッドのテストを追加します
• 既存のテストを決して失敗させません

マイグレーションの安全性

リファクタリングにマイグレーションが含まれる場合は、本番環境での安全性、データの保持、可逆性を確認します。

安全性を確保するツール

推奨 gem：

# Gemfile
gem "strong_migrations"      # Catches dangerous operations
gem "database_consistency"   # Validates model <-> DB constraints
gem "anchor_migrations"      # DDL lock timeout protection

可逆性

# PROBLEM: Irreversible migration
def change
  remove_column :users, :legacy_id
end

# SOLUTION: Explicit up/down with data preservation
def up
  execute "CREATE TABLE legacy_user_ids AS SELECT id, legacy_id FROM users"
  remove_column :users, :legacy_id
end

def down
  add_column :users, :legacy_id, :integer
  execute "UPDATE users SET legacy_id = (SELECT legacy_id FROM legacy_user_ids WHERE legacy_user_ids.id = users.id)"
end

安全なカラム操作

# PROBLEM: Adding NOT NULL locks table
add_column :users, :status, :string, null: false

# SOLUTION: Three-step migration
add_column :users, :status, :string
User.update_all(status: 'active')
change_column_null :users, :status, false

時間のかかる操作

# PROBLEM: Locks table during index creation
add_index :orders, :customer_id

# SOLUTION: Concurrent index (PostgreSQL)
disable_ddl_transaction!
add_index :orders, :customer_id, algorithm: :concurrently

データ損失のシナリオ

操作	リスク	安全な代替策
カラムの型を変更する	切り捨て	新しいカラムを追加し、データを移行してから古いカラムを削除する
カラムを削除する	データ損失	先にアーカイブしてから削除する
カラム名を変更する	アプリのエラー	追加し、データをバックフィルしてから削除する
精度を変更する	データ損失	拡張のみとし、縮小しない

マイグレーションレビューのチェックリスト

• マイグレーションは可逆ですか？ または明示的な down がありますか？
• 破壊的な変更の前にデータは保持されていますか？
• 時間のかかる操作では algorithm: :concurrently を使っていますか？
• NOT NULL は安全に追加されていますか（nullable -> backfill -> constrain）？
• strong_migrations は成功していますか？
• DDL のロックタイムアウトは設定されていますか？

出力形式

リファクタリング後、次の内容を提示します：

1. 概要 - リファクタリングした内容とその理由
2. 変更点 - 主な変更内容を含む変更ファイル
3. テスト状況 - テストが引き続き成功していることの確認
4. 警告 - 潜在的な問題や必要なフォローアップ