#ooc_2024 ©2024 RAKUS Co., Ltd. オブジェクト指向コードレビューの 新しいアプローチ Object-Oriented Conference 2024 2024/03/24 @akkiee76

#ooc_2024 ● Akihiko Sato ● 𝕏 : @akkiee76 ● Speaker Deck : @akkie76 自己紹介 懇親会を楽しみにしています 🍻 2

#ooc_2024 お知らせ 公式ガイドブック 第 16 章 「コードレビューガイドラインを 導入してみよう」 を執筆しました 🎉 3

#ooc_2024 このセッションのゴール オブジェクト指向観点のコードレビューが 明日から実践できる ✅ 4

#ooc_2024 アジェンダ 1. オブジェクト指向コードレビューの手法 2. コードレビューを支援する AI の応用 5

#ooc_2024 1. オブジェクト指向コードレビューの手法 6

#ooc_2024 導入の背景（当時のチーム課題） ● コードレビューの観点が属人的になっている ○ 品質への影響 ○ レビュアーの育成 ● 技術育成が停滞している ○ 生産性への影響 7

#ooc_2024 チーム全体が一貫性のあるコードレビューができないか？ コードレビューガイドラインを作成して 定量的なレビューを目指そう！ 8

#ooc_2024 どんなガイドラインを作って良いか見当もつかない 🤔 Google's Engineering Practices documentation を参考にすることに https://github.com/google/eng-practices 9

#ooc_2024 ここからはガイドラインに 明文化した内容を紹介します 📝 10

#ooc_2024 コードレビューの7つの観点 観点 概要 Design 設計、アーキテクチャ Simplicity 理解容易性、可読性 Naming クラス、メソッド、変数などの命名 Style コードスタイル Functionality 機能要件の充足 Test テストコードの妥当性 Document コメント、ドキュメントの妥当性 11

#ooc_2024 ここからは各観点ごとに ● 定義 ● 使用例 ガイドラインに記載した内容を紹介していきます。 12

#ooc_2024 1. Design 定義 アプリケーションとして一貫性のあるアーキテクチャ、 設計となっているか。また、アプリケーションにとって 適切な責務、振る舞いになっているか。 設計、アーキテクチャに関する指摘 13

#ooc_2024 1. Design Design 指摘に該当するケース ● アーキテクチャ原則違反 ● 単一責任の原則違反 ● 密結合 ● 低凝集 など 14

#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

#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

#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

#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

#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

#ooc_2024 2. Simplicity 定義 処理ができるだけシンプルな振る舞いになっているか。 可読性のあるコーディングになっているか。 理解容易性、可読性に関する指摘 20

#ooc_2024 2. Simplicity Simplicity 指摘に該当するケース ● 分岐が多い / ネストが深い if 文 ● 複雑な三項演算子文 ● stream, filter, map を多用した Object 整形文 ● 冗長な SQL など 21

#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

#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

#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; }

#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

#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

#ooc_2024 3. Naming 定義 変数やクラス、メソッドに責務を意図した明確な名前が付けられて いるか。英語文法に誤りがないか。 typo もこれに含まれる。 クラス、メソッド、変数などの命名に関する指摘 27

#ooc_2024 3. Naming Naming 指摘に該当するケース ● 振る舞いと一致しない変数名 ● 責務と一致しない関数名 ● 命名規則の誤り ● 英文法の誤り など 28

#ooc_2024 命名のポイント ● なるべく具体的に（抽象的でない） ● なるべく端的に（冗長でない） ● なるべく意味の範囲を狭める ● なるべくドメインに特化した命名に 29

#ooc_2024 4. Style（Code Style） 定義 コードスタイルが言語仕様、プロジェクトのルールに準拠している か。 コードスタイルに関する指摘 30

#ooc_2024 4. Style（Code Style） Style 指摘に該当するケース ● 静的解析違反 ● 不適切なアクセス修飾子 ● コーディング規約違反 など 31

#ooc_2024 静的解析違反に対する対応 コードスタイルについては議論が発生しやすので、 ● Linter ● Formatter を導入して機械的にルールを縛ってしまうのがオススメ 🚀 32

#ooc_2024 5. Functionality 定義 システムとして外部仕様を充足しているか。作者が意図した通り の振る舞いであるか。 また、通信量、パフォーマンスなどに懸念がないか。 機能要求に関する指摘 33

#ooc_2024 5. Functionality Functionality 指摘に該当するケース ● 外部機能が充足していない（不具合がある） ● 設計書の機能と異なっている ● パフォーマンスの低下が懸念される ● 不要なデータが API で送信されている など 34

#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

#ooc_2024 6. Test 定義 システムとして適切な自動テストを兼ね備えているか。自動テスト の内容で品質を担保できているか。また、システムを担保するパ ラメータ群を備えているか。 テストコードの妥当性に関する指摘 36

#ooc_2024 6. Test Test 指摘に該当するケース ● 対象のメソッドがテストされていない ● テストパターンが網羅できていない ● 定数がテストクラスに再定義されている など 37

#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

#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

#ooc_2024 7. Document 定義 ソースコード上に記載されている Doc、コメントが適切な内容であ るか。また、関連するドキュメントは更新されている か。その内容 は適切か。 コメント、ドキュメントに関する指摘 40

#ooc_2024 7. Document Document 指摘に該当するケース ● Doc やコメントの内容が不適切、不正確 ● README などの関連ドキュメントの更新漏れ など 41

#ooc_2024 コードレビューでの実践方法 🛠 42

#ooc_2024 コードレビュー時の判断基準 基準 概要 MUST マージするには必ず修正が必要 SHOULD マージ可能だが、リリースまでに修正が必要 IMO レビュアー観点の意見。修正不要 NITS IMOより軽微な意見など。修正不要 43

#ooc_2024 具体的な利用方法 コードレビューでコメントする時は、4つの判断基準と7つの観点を組み 合わせた Prefix をコメントに付けて使用します。 使用例 MUST(Design): ドメインロジックがControllerクラスに実装されて ます。 domain層の対象packageに新しくクラスを作成して実装を 移してください。 44

#ooc_2024 ここまでのまとめ 7つのレビュー観点と4つの判断基準の紹介をしました。 ガイドラインとして運用する場合は、チームに合わせて 作成することをオススメします 🚀 45

#ooc_2024 2. コードレビューを支援する AI の応用 46

#ooc_2024 AI コードレビューの準備 AI コードレビューを行うには以下のツールを利用します。 ● ChatGPT ● PR-Agent ● GitHub Actions 47 https://github.com/Codium-ai/pr-agent

#ooc_2024 PR-Agent とは？ Pull Request を AI でレビューを行えるツール ● PRの概要作成（ /describe ） ● コードレビュー（ /review ） ● 改善提案（ /improve ） Pull Request 作成時にこれらの実行が可能 🤖 48

#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

#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

#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

#ooc_2024 コードレビューを依頼してみましょう 🤖 52

#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

#ooc_2024 Pull Request 概要 54 妥当な変更内容

#ooc_2024 PR Review コードの複雑性が示唆 55

#ooc_2024 PR Code Suggestions 56 レビューの観点が考慮されている

#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

#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

#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

#ooc_2024 AI コードレビューまとめ PR-Agent とレビュー観点をプロンプトに利用した AI コードレビューの手法を紹介しました。 OpenAI を利用したツールは発展途上の部分もありますが、 人によるコードレビューコスト削減に期待。 60

#ooc_2024 このセッションのまとめ オブジェクト指向コードレビューについて紹介しました。 ● コードレビューでオブジェクト指向が学べる ● 苦手分野が明らかになる ● 技術育成のアクションプランが検討しやすい 明文化にはこのようなメリットがありますので、 チームで観点や基準を検討してぜひ実践してみてください 🚀 61

#ooc_2024 発展編 レビューガイドラインを利用した KPI 戦略 ● API を利用してレビューコメントを可視化 ● 弱点分析 & アクション検討 GitLab API で Merge Request のコメントを一括取得する方法 62

#ooc_2024 Thank you 🙏 63