明日から始めるリファクタリング

チームと個人のリファクタリング実践談 / 関ジャバ'25 9月度 でお話しした内容です

Transcript

1. 明日から始めるリファクタリング
2. 目次 リファクタリングとは 「コードの不吉な臭い」を見つけよう 初級編 中級編 安全なリファクタリングの2つの原則 チームで実践するためのポイント まとめ
3. いくつかリファクタリングをした結果のコードがありますが、それが最適解ではあり ません。リファクタリングの一部の手法を適用した途中段階のものやコード量の都合 上、全ての実装を書いていない例があります。
4. リファクタリングとは
5. 機能開発の理想と懸念 普段の開発において...... 理想: より理解しやすい整理されたコードにしたい 現実の懸念 現在動作しているコードを変更することのリスク 新たなバグを引き起こすかもしれない コードをきれいにすることの時間コスト 新機能の実装が遅くなるかもしれない → これらの懸念に対して現実的な解決策を提供するのが リファクタリング
6. リファクタリングの定義 ソフトウェアの外部の振る舞いを保ったままで、内部の構造を改善していく作業 → 正しくリファクタリングを行えば、ソフトウェアの動作は変わらず、バグを生み出 すリスクを少なくできる
7. リファクタリングの目的 最大の目的: 将来の開発速度を上げること ソフトウェア設計を改善 コードを理解しやすくする バグを見つけやすくする → リファクタリングは未来への投資
8. 具体例: 通知機能の拡張 状況 メール通知機能を持つシステム 新要求: LINE通知機能を追加したい
9. 現状のコード public class OrderService { public void processOrder(Order order) { // 注文処理... String body = "ご注文ありがとうございます。注文ID: " + order.getId(); // メール送信処理が直接書かれている new EmailClient().send(order.getCustomerEmail(), "注文確認", body); } } public class UserService { public void registerUser(User user) { // ユーザー登録処理... String body = "ユーザー登録が完了しました。ID: " + user.getId(); // 同じメール送信処理がコピペされている new EmailClient().send(user.getEmail(), "登録完了", body); } }
10. 現在の問題 通知処理が各所に直接書かれている → 重複と分岐が発生 仕様変更で修正箇所が増える
11. リファクタリングなしで対応すると...... public class OrderService { public void processOrder(Order order, boolean useLine) { String body = "ご注文ありがとうございます。注文ID: " + order.getId(); if (useLine) { new LineClient().send(order.getCustomerLineId(), body); } else { new EmailClient().send(order.getCustomerEmail(), "注文確認", body); } } } public class UserService { public void registerUser(User user, boolean useLine) { String body = "ユーザー登録が完了しました。ID: " + user.getId(); if (useLine) { new LineClient().send(user.getLineId(), body); } else { new EmailClient().send(user.getEmail(), "登録完了", body); } } }
12. コピペでの対応が将来的な負債に 問題点 通知フォーマット変更時に以下のようなデメリットがあり、開発速度が低下 複数箇所を修正する必要 修正漏れや不整合のリスク → リファクタリングでコードを理解・修正しやすくすることが重要
13. 例: 通知の責任を分離 public interface NotificationService { void sendOrderConfirmation(Order order); void sendUserRegistrationConfirmation(User user); } public class OrderService { private final NotificationService notificationService; public void processOrder(Order order) { // 注文処理... notificationService.sendOrderConfirmation(order); } } public class UserService { private final NotificationService notificationService; public void registerUser(User user) { // ユーザー登録処理... notificationService.sendUserRegistrationConfirmation(user); } }
14. 「コードの不吉な臭い」を見つけよう
15. 「明日から」できるリファクタリングのポイント「不吉な臭い」を紹介 「不吉な臭い」とは？ リファクタリングが必要な箇所を示すサイン 「リファクタリング　既存のコードを安全に改善する（第2版） 」で紹介
16. 初級編 日常のコーディングの中で比較的簡単に見つけて改善できるポイント 長い関数 不可思議な名前 コメント
17. 長い関数 どんな臭い？ 関数が非常に長く、処理内容が多岐にわたり、一見して何をしているのか分 かりにくい状態 なぜ問題？ コードの理解に時間がかかり、可読性を低下させる 改善策 「関数の抽出」 意味のあるまとまりを新しい関数として抽出し、 「どうやるかではなく、 何をするか」を端的に示す名前を付ける
18. 具体例: 注文処理を行う関数の例
19. Before: 一つの関数に多くの処理が詰め込まれている public class OrderProcessor { public void processOrder(String customerName, List<Item> items) { if (customerName == null || customerName.isEmpty()) { throw new IllegalArgumentException("顧客名が必要です"); } for (Item item : items) { if (item.getStock() < item.getQuantity()) { throw new RuntimeException("在庫不足: " + item.getName()); } } double total = 0; for (Item item : items) { total += item.getPrice() * item.getQuantity(); } double shipping = total > 5000 ? 0 : 500; System.out.println("注文確定: " + customerName); System.out.println("合計: " + (total + shipping) + "円"); } }
20. After: 意味のある単位で 関数の抽出 を適用 public class OrderProcessor { public void processOrder(String customerName, List<Item> items) { validateCustomer(customerName); checkStock(items); double total = calculateTotal(items); double shipping = calculateShipping(total); confirmOrder(customerName, total + shipping); } private void validateCustomer(String customerName) { if (customerName == null || customerName.isEmpty()) { throw new IllegalArgumentException("顧客名が必要です"); } } private void checkStock(List<Item> items) { ...... } private double calculateTotal(List<Item> items) { ...... } private double calculateShipping(double total) { ...... } private void confirmOrder(String customerName, double finalAmount) { ...... } }
21. 不可思議な名前 どんな臭い？ 関数、変数、クラスなどの名前が、その 「意図」や「目的」をはっきり伝え ない状態 なぜ問題？ コードを読んだ人が、その要素が何をしているのかを推測するのに時間を費 やす コードの意図が不明瞭なため、誤った使い方をしてしまう可能性や、バグを 埋め込むリスクが高まる 改善策 「関数宣言の変更」 「変数名の変更」 「フィールド名の変更」
22. 具体例: 消費税計算を行う関数の例
23. Before: calc (何を計算するのか不明瞭) public class ShoppingCart { public double calc(int quantity, double price) { return quantity * price * 0.1; // 消費税？割引？ } public double processOrder() { return calc(3, 1000); // 何の計算？ } }
24. After: calculateTax に関数名を変更 public class ShoppingCart { static final double TAX = 0.1; public double calculateTax(int quantity, double price) { return quantity * price * TAX; } public double processOrder() { return calculateTax(3, 1000); // 消費税計算 } } 良い名前が思いつかないということは、設計がまだ固まっていないことの兆候で もあります。今一つな名前をどうにかしようと頭をひねることで、コードがずっ とシンプルになっていくこともあります。
25. コメント どんな臭い？ コメント自体は不吉な臭いではない しかし、コメントが不明瞭なコードを補うための「消臭剤」として使われる 場合、それが不吉な臭いの予兆となる なぜ問題？ そのコード自体が意図を明確に伝えていないことを示す 改善策 「関数の抽出」: コメントで説明している処理を関数名にする 「関数宣言の変更」: 意図が明確になる名前／引数／戻り値にリネーム
26. 具体例: 複雑な条件分岐の前に説明コメントがある例
27. Before: 複雑な条件分岐の前に、その意図を説明するコメントがある。 public class ShippingValidator { public boolean canShip(Order order) { // 合計金額が1000円以上で送料無料、総重量が30kg未満、全ての商品が在庫あり double total = 0; int weight = 0; for (Item i : order.getItems()) { total += i.getPrice() * i.getQuantity(); weight += i.getWeight() * i.getQuantity(); if (i.getStock() < i.getQuantity()) return false; } return total >= 1000 || weight < 30; } } マジックナンバーやビジネスルールがコメントなしでは理解が難しい
28. After: コメントで説明されている条件分岐に対して 関数の抽出 を適用 public class ShippingValidator { private static final double FREE_SHIPPING_THRESHOLD = 1000.0; private static final int MAX_WEIGHT_KG = 30; public boolean canShip(Order order) { return allItemsInStock(order) && (isFreeShipping(order) || isWithinWeightLimit(order)); } private boolean allItemsInStock(Order order) { return order.getItems().stream().allMatch(i -> i.getStock() >= i.getQuantity()); } private boolean isFreeShipping(Order order) { return calculateTotal(order) >= FREE_SHIPPING_THRESHOLD; } private boolean isWithinWeightLimit(Order order) { return calculateWeight(order) < MAX_WEIGHT_KG; } private double calculateTotal(Order order) { return order.getItems().stream().mapToDouble(i -> i.getPrice() * i.getQuantity()).sum(); } private int calculateWeight(Order order) { return order.getItems().stream().mapToInt(i -> i.getWeight() * i.getQuantity()).sum(); } }
29. これら初級編は、日常のコーディングの中に取り入れやすいポイント 私は git で commit する前に diff を眺めて気になるポイントを探すようにしている
30. 中級編 次のステップとして以下の「不吉な臭い」にも注意を払ってみると良い データの群れ 基本データ型への執着
31. データの群れ どんな臭い？ 数個のデータが常に一緒に、クラスのフィールドやメソッドのシグネチャな ど、さまざまな箇所に現れる状態 なぜ問題？ データのまとまりに意味があるにもかかわらず、それが明示的な構造として 表現されていないため、コードの可読性を低下させ、重複を生み出しやすく する 特定のデータ群を変更する必要がある場合、すべての出現箇所を見つけて修 正する必要があり、不整合が生じるリスクが高まる 改善策 「クラスの抽出」 ：データの群れを一つのオブジェクト（クラス）にまとめる
32. 具体例: 顧客の連絡先情報が複数箇所で個別に扱われている
33. Before: 顧客情報とイベント申込で、連絡先データが個別のフィールドとして重複 public class Customer { private String name; private String email; private String phone; private String zipCode; private String prefecture; private String city; private String address; } // 同じような連絡先データが重複して定義されている public class EventRegistration { private String eventId; private String participantEmail; private String participantPhone; private String emergencyZipCode; private String emergencyPrefecture; private String emergencyCity; private String emergencyAddress; }
34. After: 連絡先情報を ContactInfo として クラスの抽出 を適用 public class ContactInfo { private String email; private String phone; private String zipCode; private String prefecture; private String city; private String address; } public class Customer { private String name; private ContactInfo contactInfo; } public class EventRegistration { private String eventId; private ContactInfo emergencyContact; } ※ モデリング次第では他の抽出方法もあり得る
35. 基本データ型への執着 どんな臭い？ 金額、座標、期間、電話番号といったドメイン固有の概念を、単なる整数、 文字列などの基本データ型で表現している状態 なぜ問題？ ドメイン上の意味が失われ、不適切な操作やバグの原因となる（例: 異なる通 貨の金額を直接足し算する） 関連する振る舞い（フォーマット、バリデーション、比較など）がコードの あらゆるところに書かれ、一貫性を保つのが困難に 改善策 「オブジェクトによるプリミティブの置き換え」 ：基本データ型で表されてい る値を、その値と振る舞いを持つ専用のクラスに置き換える
36. 具体例: 注文の優先度を文字列で管理している例
37. Before: 優先度を文字列 ( "high" , "rush" ) で直接比較している。 public class Order { private String priority; // 優先度を文字列で管理。ドメイン上の意味が不明瞭 public String getPriority() { return priority; } public void setPriority(String priority) { this.priority = priority; } } public class ClientCode { public long countHighPriorityOrders(List<Order> orders) { // クライアントコード: 文字列で優先度を直接比較。ドメインロジックが散在している return orders.stream() .filter(o -> "high".equals(o.getPriority()) || "rush".equals(o.getPriority())) .count(); } public static void main(String[] args) { List<Order> orders = new ArrayList<>(); orders.add(new Order("low")); orders.add(new Order("normal")); orders.add(new Order("high")); orders.add(new Order("rush")); new ClientCode().countHighPriorityOrders(orders); } }
38. After: Priority として オブジェクトによるプリミティブの置き換え を適用 public enum Priority { LOW(0), NORMAL(1), HIGH(2), RUSH(3); private final int level; Priority(int level) { this.level = level; } public boolean higherThan(Priority other) { return this.level > other.level; } }
39. public class Order { private Priority priority; // Priorityオブジェクトを保持 public Order(Priority priority) { this.priority = priority; } } public class ClientCode { public long countHighPriorityOrders(List<Order> orders) { return orders.stream() .map(Order::getPriority) .filter(p -> p.higherThan(Priority.NORMAL)) .count(); } } 優先度に関するロジックが Priority 内にカプセル化された
40. 初級編が手続き（メソッド）の整理だったのに対し 中級編はデータと振る舞いをまとめてオブジェクトとして扱う オブジェクト指向を意識したリファクタリング
41. 安全なリファクタリングの2つの原則
42. 安全なリファクタリングの2つの原則 リファクタリングの過程でバグを混入させないために、以下の2つの原則を守ることが 重要。 原則1： 堅牢なテスト群を用意する 原則2： 小さなステップで進める
43. 原則1： 堅牢なテスト群を用意する リファクタリングは「振る舞いを変えない」ことが大前提 どんなに注意してもバグ混入の可能性はゼロにはならない 堅牢なテストスイートが意図せぬ変更やバグをすぐに指摘
44. 効果的なテスト群の3つの条件 1. 自動化されていること 全てのテストを完全に自動化し、テスト自身に結果をチェックさせる 2. 高頻度で実行できること すぐに実行でき、すぐに結果がわかる (参考: 作業中のコードについては少なくとも 2〜3 分に 1 回実行、少なくとも 1 日 に 1 回は全てのテストを実行と書籍に記載されている) 3. リスクのある部分を重点的にカバーしていること コードを見て、複雑になっているところに注目する 機能を見て、エラーが起こりそうなところを考える 境界条件（空のリストや null 値など）をテストする
45. 身につけたい習慣 1. バグ修正時はテストから書く バグレポートを受け取ったら、まずそのバグを再現するテストを書く その後でコードを修正し、テストが通ることを確認する 2. テストは「振る舞い」をチェックする 外部から観察可能な振る舞いをテストする 内部実装の詳細（privateメソッドなど）はテスト対象外でOK 3. 失敗するテストがある時はリファクタリングしない まず全てのテストが通るようにしてからリファクタリングを開始する
46. 原則2：小さなステップで進める 一度に大きな変更はしない 「関数の抽出」のような小さなリファクタリングを一つ行い、 「コンパイル → テスト → コミット」を繰り返すのが理想 小さく進めることのメリット もし問題が起きても、直前の変更を戻すだけなので、安全に作業を進められる 小さく壊れない単位で進めることで、途中で中断することが可能 コードが壊れる期間を非常に短くできる コードが壊れた状態になるリスクを最小限にできる
47. チームで実践するためのポイント
48. チームで実践するためのポイント 私のチームでリファクタリングを進めていく上で意識しているポイントを紹介 安全なリファクタリングの原則を守る 機能追加後の振り返りで開発しづらかったポイントを共有する リファクタリング後のコードベースの認識をそろえる チームメンバー誰でも着手可能な小さなタスクに分解する
49. 1. 安全なリファクタリングの原則を守る テスト群があることで自信を持って取り組める 小さなステップで途中で中断可能、新機能開発と両立しやすい コードベースが短期間で大きく変わらない → チームの生産性を維持
50. 2. 機能追加後の振り返りで開発しづらかったポイントを共有 大きめの機能開発後に30分程度の振り返り 「今回の変更で開発しづらかったポイント」を共有 具体例 「このクラスは責務が多すぎる」 「このデータ構造は複雑で扱いづらかった」 → リファクタリングの必要性がチーム全体で認識される
51. 3. リファクタリング後のコードベースの認識をそろえる 振り返りで見つかったポイントに対して、理想の形をメンバー間で認識統一 具体例 「このクラスはもっと小さく分割したい」 「このデータ構造はもっとシンプルにしたい」 → リファクタリングの方向性が明確になり、認識ズレを防ぐ
52. 4. チームメンバー誰でも着手可能な小さなタスクに分解 理想形を実現するリファクタリングの流れを小さなステップに分解 「小サイズ」のタスクとしてタスクボードに登録 → メンバーが自分のタイミングでタスクを取ってリファクタリングを実行
53. まとめ
54. まとめ リファクタリングはポイントを守って取り組むことで、日々の活動に組み込むことが できる未来への投資。 実践の3ステップ Step 1: 自動テストを整備 スコープを小さく絞って、テストを追加 Step 2: コミット前に「コードの不吉な臭い」チェック diffで「長い関数」 「不可思議な名前」をチェック Step 3: チーム振り返りでポイント共有 機能開発後に「開発しづらかった箇所」を共有
55. 参考文献 リファクタリング　既存のコードを安全に改善する（第2版）