オブジェクト指向コードレビューの新しいアプローチ

Object-Oriented Conference 2024 の「オブジェクト指向コードレビューの 新しいアプローチ」セッションの資料です。

アジェンダ
・オブジェクト指向コードレビューの手法
・コードレビューを強化する AI の応用

【Object-Oriented Conference 2024】
https://ooc.dev/2024/

【プロポーザル】
https://fortee.jp/oocon-2024/proposal/cfa1a92c-5659-4941-b9bc-c408bf3bd886

Transcript

1. #ooc_2024 ©2024 RAKUS Co., Ltd. オブジェクト指向コードレビューの 新しいアプローチ Object-Oriented Conference 2024 2024/03/24 @akkiee76
2. #ooc_2024 • Akihiko Sato • 𝕏 : @akkiee76 • Speaker Deck : @akkie76 自己紹介 懇親会を楽しみにしています 🍻 2
3. #ooc_2024 お知らせ 公式ガイドブック 第 16 章 「コードレビューガイドラインを 導入してみよう」 を執筆しました 🎉 3
4. #ooc_2024 このセッションのゴール オブジェクト指向観点のコードレビューが 明日から実践できる ✅ 4
5. #ooc_2024 アジェンダ 1. オブジェクト指向コードレビューの手法 2. コードレビューを支援する AI の応用 5
6. #ooc_2024 1. オブジェクト指向コードレビューの手法 6
7. #ooc_2024 導入の背景（当時のチーム課題） • コードレビューの観点が属人的になっている ◦ 品質への影響 ◦ レビュアーの育成 • 技術育成が停滞している ◦ 生産性への影響 7
8. #ooc_2024 チーム全体が一貫性のあるコードレビューができないか？ コードレビューガイドラインを作成して 定量的なレビューを目指そう！ 8
9. #ooc_2024 どんなガイドラインを作って良いか見当もつかない 🤔 Google's Engineering Practices documentation を参考にすることに https://github.com/google/eng-practices 9
10. #ooc_2024 ここからはガイドラインに 明文化した内容を紹介します 📝 10
11. #ooc_2024 コードレビューの7つの観点 観点 概要 Design 設計、アーキテクチャ Simplicity 理解容易性、可読性 Naming クラス、メソッド、変数などの命名 Style コードスタイル Functionality 機能要件の充足 Test テストコードの妥当性 Document コメント、ドキュメントの妥当性 11
12. #ooc_2024 ここからは各観点ごとに • 定義 • 使用例 ガイドラインに記載した内容を紹介していきます。 12
13. #ooc_2024 1. Design 定義 アプリケーションとして一貫性のあるアーキテクチャ、 設計となっているか。また、アプリケーションにとって 適切な責務、振る舞いになっているか。 設計、アーキテクチャに関する指摘 13
14. #ooc_2024 1. Design Design 指摘に該当するケース • アーキテクチャ原則違反 • 単一責任の原則違反 • 密結合 • 低凝集 など 14
15. #ooc_2024 単一責任原則違反例 fun add(product: Product): Boolean { if (product.id < 0) { throw IllegalArgumentException() } if (product.name.isEmpty()) { throw IllegalArgumentException() } val tempPrice: Int = if (product.canDiscount) { manager.totalPrice + manager.getDiscountPrice(product.price) } else { manager.totalPrice + product.price } return if (tempPrice < 3000) { manager.totalPrice = manager.discountProducts.add(product) true } else { false } } fun add(product: Product): Boolean { if (product.id < 0) { // バリデーション 1 throw IllegalArgumentException() } if (product.name.isEmpty()) { // バリデーション 2 throw IllegalArgumentException() } val tempPrice: Int = if (product.canDiscount) { // 条件分岐 1 manager.totalPrice + manager.getDiscountPrice(product.price) } else { manager.totalPrice + product.price } return if (tempPrice < 3000) { // 条件分岐 2 manager.totalPrice = manager.discountProducts.add(product) true } else { false } } 4 つの責務を持つ関数 15
16. #ooc_2024 fun add(product: Product) { validateProduct(product) if (canAdd(product)) { manager.totalPrice = manager.discountProducts.add(product) } } private fun validateProduct(product: Product) { // バリデーション if (product.id < 0 || product.name.isEmpty()) { throw IllegalArgumentException() } } private fun canAdd(product: Product): Boolean { // 条件分岐 val tempPrice = getTempPrice(product) return tempPrice >= 3000 } private fun getTempPrice(product: Product): Int { // 条件分岐 return if (product.canDiscount) { manager.totalPrice + manager.getDiscountPrice(product.price) } else { manager.totalPrice + product.price } } Design 指摘で責務分離を推奨 16
17. #ooc_2024 密結合例 class Calculator { private val taxCalculator = TaxCalculator() fun calculateTotalPrice(price: Double): Double { val tax = taxCalculator.calculateTax(price) return price + tax } } class TaxCalculator { fun calculateTax(price: Double): Double { // 価格の10%を税金として計算する return price * 0.1 } } val calculator = Calculator() val totalPrice = calculator.calculateTotalPrice(100.0) TaxCalculatorクラスに依 存している 17
18. #ooc_2024 class Calculator { private val taxCalculator = TaxCalculator() fun calculateTotalPrice(price: Double): Double { val tax = taxCalculator.calculateTax(price) return price + tax } } class TaxCalculator { fun calculateTax(price: Double): Double { // 価格の20%を税金として計算する return price * 0.1 return price * 0.2 } } val calculator = Calculator() val totalPrice = calculator.calculateTotalPrice(100.0) 密結合例 ロジックが変更されると 予期せぬ影響が・・・ Design 指摘で疎結合を推奨 18
19. #ooc_2024 class Calculator(private val taxCalculator: TaxCalculator) { fun calculateTotalPrice(price: Double): Double { val tax = taxCalculator.calculateTax(price) return price + tax } } class TaxCalculator { fun calculateTax(price: Double): Double { // 価格の10%を税金として計算する return price * 0.1 } } val taxCalculator = TaxCalculator() val calculator = Calculator(taxCalculator) val totalPrice = calculator.calculateTotalPrice(100.0) 密結合例 依存関係を注入し 疎結合へ設計変更を推奨 19
20. #ooc_2024 2. Simplicity 定義 処理ができるだけシンプルな振る舞いになっているか。 可読性のあるコーディングになっているか。 理解容易性、可読性に関する指摘 20
21. #ooc_2024 2. Simplicity Simplicity 指摘に該当するケース • 分岐が多い / ネストが深い if 文 • 複雑な三項演算子文 • stream, filter, map を多用した Object 整形文 • 冗長な SQL など 21
22. #ooc_2024 分岐が多い if 文例 val powerRate: Float = member.powerRate / menber.maxPowerRate var currentCondition: Condition = Condition.DEFAULT if (powerRate == 0) { currentCondition = Condition.DEAD } else if (powerRate < 0.3) { currentCondition = Condition.DANGER } else if (powerRate < 0.5) { currentCondition = Condition.WARNING } else { currentCondition = Condition.GOOD } return currentCondition val powerRate: Float = member.powerRate / menber.maxPowerRate var currentCondition: Condition = Condition.DEFAULT if (powerRate == 0) { // 分岐 1 currentCondition = Condition.DEAD } else if (powerRate < 0.3) { // 分岐 2 currentCondition = Condition.DANGER } else if (powerRate < 0.5) { // 分岐 3 currentCondition = Condition.WARNING } else { // 分岐 4 currentCondition = Condition.GOOD } return currentCondition 早期 return でネストの解 消を推奨 22
23. #ooc_2024 分岐が多い if 文例 val powerRate: Float = member.powerRate / menber.maxPowerRate if (powerRate == 0) { return Condition.DEAD } if (powerRate < 0.3) { return Condition.DANGER } if (powerRate < 0.5) { return Condition.WARNING } return Condition.GOOD 早期 return でネストを解消し可 読性アップ！ 23
24. #ooc_2024 複雑な三項演算子例 int compare(int a, int b, int c) { return (a > b) ? ((a > c) ? a : c) : ((b > c) ? b : c); } int compare(int a, int b, int c) { if (a > b) { if (a > c) { return a; } } else { if (b > c) { return b; } } return c; } int compare(int a, int b, int c) { return maxOf(a, maxOf(b, c)); } int maxOf(int value1, int value2) { return Math.max(value1, value2); } 理解容易性が向上 Math.max で 理解容易性が向上 int compare(int a, int b, int c) { return (a > b) ? ((a > c) ? a : c) : ((b > c) ? b : c); } 直感的に分かりにくい 三項演算子 24 int compare(int a, int b, int c) { if (a > b) { if (a > c) { return a; } } else { if (b > c) { return b; } } return c; }
25. #ooc_2024 stream, filter, map を多用した Object 整形文例 val people = listOf( Person("Alice", 25, listOf(Skill("Kotlin", 3), Skill("Java", 4))), Person("Bob", 30, listOf(Skill("Kotlin", 4), Skill("Python", 3))), Person("Charlie", 20, listOf(Skill("Java", 2), Skill("JavaScript", 3))) // 以降も続く・・・ ) val targetLanguage = "Kotlin" val minSkillLevel = 4 val result = people.stream() .filter { person -> person.skills.any { it.language == targetLanguage && it.level >= minSkillLevel } } .map { it.name } .toList() 期待値が分かりにくいので 理解容易性を推奨 25
26. #ooc_2024 stream, filter, map を多用した Object 整形文例 val people = listOf( Person("Alice", 25, listOf(Skill("Kotlin", 3), Skill("Java", 4))), Person("Bob", 30, listOf(Skill("Kotlin", 4), Skill("Python", 3))), Person("Charlie", 20, listOf(Skill("Java", 2), Skill("JavaScript", 3))) // 以降も続く・・・ ) val targetLanguage = "Kotlin" val minSkillLevel = 4 val proficientInTargetLanguage = { person: Person -> person.skills.any { it.language == targetLanguage && it.level >= minSkillLevel } } val result = people .filter(proficientInTargetLanguage) .map { it.name } .toList() 理解容易性が向上 26
27. #ooc_2024 3. Naming 定義 変数やクラス、メソッドに責務を意図した明確な名前が付けられて いるか。英語文法に誤りがないか。 typo もこれに含まれる。 クラス、メソッド、変数などの命名に関する指摘 27
28. #ooc_2024 3. Naming Naming 指摘に該当するケース • 振る舞いと一致しない変数名 • 責務と一致しない関数名 • 命名規則の誤り • 英文法の誤り など 28
29. #ooc_2024 命名のポイント • なるべく具体的に（抽象的でない） • なるべく端的に（冗長でない） • なるべく意味の範囲を狭める • なるべくドメインに特化した命名に 29
30. #ooc_2024 4. Style（Code Style） 定義 コードスタイルが言語仕様、プロジェクトのルールに準拠している か。 コードスタイルに関する指摘 30
31. #ooc_2024 4. Style（Code Style） Style 指摘に該当するケース • 静的解析違反 • 不適切なアクセス修飾子 • コーディング規約違反 など 31
32. #ooc_2024 静的解析違反に対する対応 コードスタイルについては議論が発生しやすので、 • Linter • Formatter を導入して機械的にルールを縛ってしまうのがオススメ 🚀 32
33. #ooc_2024 5. Functionality 定義 システムとして外部仕様を充足しているか。作者が意図した通り の振る舞いであるか。 また、通信量、パフォーマンスなどに懸念がないか。 機能要求に関する指摘 33
34. #ooc_2024 5. Functionality Functionality 指摘に該当するケース • 外部機能が充足していない（不具合がある） • 設計書の機能と異なっている • パフォーマンスの低下が懸念される • 不要なデータが API で送信されている など 34
35. #ooc_2024 パフォーマンス比較例 // 金額をカンマ区切りに変換する fun formatAmount1(amount: Int): String { val amountStr = amount.toString() var formattedAmount = "" var count = 0 for (i in amountStr.length - 1 downTo 0) { formattedAmount = amountStr[i] + formattedAmount count++ if (count % 3 == 0 && i != 0) { formattedAmount = ",$formattedAmount" } } return formattedAmount } fun formatAmount2(amount: Int): String { return amount.toString().replace(Regex("(\\d)(?=(\\d{3})+(?!\\d))"), "$1,") } 1 回あたり 0.007ms 1 回あたり 0.063ms 35
36. #ooc_2024 6. Test 定義 システムとして適切な自動テストを兼ね備えているか。自動テスト の内容で品質を担保できているか。また、システムを担保するパ ラメータ群を備えているか。 テストコードの妥当性に関する指摘 36
37. #ooc_2024 6. Test Test 指摘に該当するケース • 対象のメソッドがテストされていない • テストパターンが網羅できていない • 定数がテストクラスに再定義されている など 37
38. #ooc_2024 public class LoginController { private static final int MAX_PASSWORD_LENGTH = 10; } public class LoginControllerTest { private static final int MAX_PASSWORD_LENGTH = 10; @Test void test_validate_password() { // バリデーションの処理 assertEquals(10, MAX_PASSWORD_LENGTH); } } 定数がテストクラスに再定義されている例 private を維持するため 重複定義されてる public class LoginController { private static final int MAX_PASSWORD_LENGTH = 10; private static final int MAX_PASSWORD_LENGTH = 12; } public class LoginControllerTest { private static final int MAX_PASSWORD_LENGTH = 10; @Test void test_validate_password() { // バリデーションの処理 assertEquals(10, MAX_PASSWORD_LENGTH); } } 閾値の変更があった場合もテストが成功してしまう 😱 38
39. #ooc_2024 定数がテストクラスに再定義されている例 public class LoginController { private static final int MAX_PASSWORD_LENGTH = 10; static final int MAX_PASSWORD_LENGTH = 10; } public class LoginControllerTest { private static final int MAX_PASSWORD_LENGTH = 10; @Test void test_validate_password() { // バリデーションの処理 assertEquals(10, MAX_PASSWORD_LENGTH); } } package private で 同じ定数を 参照するように推奨 39
40. #ooc_2024 7. Document 定義 ソースコード上に記載されている Doc、コメントが適切な内容であ るか。また、関連するドキュメントは更新されている か。その内容 は適切か。 コメント、ドキュメントに関する指摘 40
41. #ooc_2024 7. Document Document 指摘に該当するケース • Doc やコメントの内容が不適切、不正確 • README などの関連ドキュメントの更新漏れ など 41
42. #ooc_2024 コードレビューでの実践方法 🛠 42
43. #ooc_2024 コードレビュー時の判断基準 基準 概要 MUST マージするには必ず修正が必要 SHOULD マージ可能だが、リリースまでに修正が必要 IMO レビュアー観点の意見。修正不要 NITS IMOより軽微な意見など。修正不要 43
44. #ooc_2024 具体的な利用方法 コードレビューでコメントする時は、4つの判断基準と7つの観点を組み 合わせた Prefix をコメントに付けて使用します。 使用例 MUST(Design): ドメインロジックがControllerクラスに実装されて ます。 domain層の対象packageに新しくクラスを作成して実装を 移してください。 44
45. #ooc_2024 ここまでのまとめ 7つのレビュー観点と4つの判断基準の紹介をしました。 ガイドラインとして運用する場合は、チームに合わせて 作成することをオススメします 🚀 45
46. #ooc_2024 2. コードレビューを支援する AI の応用 46
47. #ooc_2024 AI コードレビューの準備 AI コードレビューを行うには以下のツールを利用します。 • ChatGPT • PR-Agent • GitHub Actions 47 https://github.com/Codium-ai/pr-agent
48. #ooc_2024 PR-Agent とは？ Pull Request を AI でレビューを行えるツール • PRの概要作成（ /describe ） • コードレビュー（ /review ） • 改善提案（ /improve ） Pull Request 作成時にこれらの実行が可能 🤖 48
49. #ooc_2024 PR-Agent を利用するための準備 PR-Agentを利用するためには以下を準備します。 • OpenAI API key（従量課金設定） • GitHub personal access token • pr-agent-review.yaml ◦ https://github.com/Codium-ai/pr-agent/blob/main/.github/workflows/pr-agent-review.yaml GitHub Action から OpenAI API で gpt-4による レビューを行うため利用規約を必ずご確認ください https://github.com/Codium-ai/pr-agent?tab=readme-ov-file#data-privacy 49
50. #ooc_2024 name: PR-Agent on: pull_request: types: [opened, reopened, synchronize] # issue_comment: workflow_dispatch: permissions: # issues: write pull-requests: write jobs: pr_agent_job: runs-on: ubuntu-latest name: Run pr agent on every pull request steps: - name: PR Agent action step id: pragent uses: Codium-ai/pr-agent@main env: GITHUB_ACTION_CONFIG.AUTO_DESCRIBE: true GITHUB_ACTION_CONFIG.AUTO_REVIEW: true GITHUB_ACTION_CONFIG.AUTO_IMPROVE: true 実行タイミングを追加 PRへの書込み権限を付与 describe, review, improve を有効にする 50
51. #ooc_2024 jobs: pr_agent_job: env: INSTRUCTIONS: >- - 日本語で回答してください - 以下の観点で指摘を分類してください - Design - 単一責任の原則違反、密結合、低凝集などの設計的な指摘 - Simplicity - 理解容易性、可読性観点の指摘 - Naming - クラス、メソッド、変数など命名が内容に相応しいかどうかの指摘 - Style - コードスタイルに問題がないかどうかの指摘 - Functionality - 実装されたコードが期待した挙動になるか、コードに不具合がないか - Test - 閾値のテストができているか、コードの分岐がテストコードで担保されているか - Document - コードに対する説明が適切かどうか - 上記の指摘観点をCategory欄に記載すること - 上記の指摘観点をprefixに付けること。Design観点の場合はコメントにDesign:とprefixを付けること steps: env: PR_REVIEWER.EXTRA_INSTRUCTIONS: ${{ env.INSTRUCTIONS }} PR_CODE_SUGGESTIONS.EXTRA_INSTRUCTIONS: ${{ env.INSTRUCTIONS }} レビュー観点の プロンプトを追加 具体的に例示する 51
52. #ooc_2024 コードレビューを依頼してみましょう 🤖 52
53. #ooc_2024 val x = 10 val y = 20 val z = 30 val result = if (x > y) { if (x > z) { if (y > z) { "x is the largest: $x" } else { "y is the largest: $y" } } else { "z is the largest: $z" } } else { if (y > z) { if (x > z) { "x is the largest: $x" } else { "z is the largest: $z" } } else { "y is the largest: $y" } } println(result) 意味のない定数名 ネストの深い 三項演算子 サンプルコードをコミットして PRを作成 🛠 53
54. #ooc_2024 Pull Request 概要 54 妥当な変更内容
55. #ooc_2024 PR Review コードの複雑性が示唆 55
56. #ooc_2024 PR Code Suggestions 56 レビューの観点が考慮されている
57. #ooc_2024 Simplicity -val result = if (x > y) { - if (x > z) { - if (y > z) { - "x is the largest: $x" - } else { - "y is the largest: $y" - } - } else { - "z is the largest: $z" - } -} else { - if (y > z) { - if (x > z) { - "x is the largest: $x" - } else { - "z is the largest: $z" - } - } else { - "y is the largest: $y" - } -} +val largest = maxOf(x, y, z) +val result = "The largest: $largest" 複数の入れ子になった if 文を使用す る代わりに、maxOf 関数を使用して コードを簡素化することを提案しま す。 これにより、コードの可読性が向上 し、保守性が高まります。 57
58. #ooc_2024 Naming -val x = 10 -val y = 20 -val z = 30 +val firstNumber = 10 +val secondNumber = 20 +val thirdNumber = 30 x、y、zの命名が一般的すぎます。 これらの変数が何を表しているのか をより明確にするために、より具体的 な名前を使用することを検討してくだ さい。 58
59. #ooc_2024 Design fun main() { val x = 10 val y = 20 val z = 30 - ... + val result = calculateLargest(x, y, z) println(result) } +fun calculateLargest(a: Int, b: Int, c: Int): String { + val largest = maxOf(a, b, c) + return "The largest: $largest" +} + この関数は単一のタスク（最大値の 計算と表示）を実行していますが、将 来的に機能が拡張される可能性を考 慮して、最大値を計算するロジックを 別の関数に分割することを検討してく ださい。 これにより、単一責任の原則に従い、 コードの再利用性とテスト容易性が 向上します。 59
60. #ooc_2024 AI コードレビューまとめ PR-Agent とレビュー観点をプロンプトに利用した AI コードレビューの手法を紹介しました。 OpenAI を利用したツールは発展途上の部分もありますが、 人によるコードレビューコスト削減に期待。 60
61. #ooc_2024 このセッションのまとめ オブジェクト指向コードレビューについて紹介しました。 • コードレビューでオブジェクト指向が学べる • 苦手分野が明らかになる • 技術育成のアクションプランが検討しやすい 明文化にはこのようなメリットがありますので、 チームで観点や基準を検討してぜひ実践してみてください 🚀 61
62. #ooc_2024 発展編 レビューガイドラインを利用した KPI 戦略 • API を利用してレビューコメントを可視化 • 弱点分析 & アクション検討 GitLab API で Merge Request のコメントを一括取得する方法 62
63. #ooc_2024 Thank you 🙏 63