Upgrade to Pro — share decks privately, control downloads, hide ads and more …

コードレビューで学ぶ!Kotlinオブジェクト指向デザインパターン

akkie76
April 02, 2024

 コードレビューで学ぶ!Kotlinオブジェクト指向デザインパターン

Kotlin Fest 2024のプロポーザルとして提案している「コードレビューで学ぶ!Kotlinオブジェクト指向デザインパターン」のスライドを一部ご紹介します。

このセッションでは、オブジェクト指向の基礎に立ち返りながら、コードレビューを通じてKotlinの設計デザインパターンを深掘りしていきます。現場で直面する様々なアンチパターンから、それを如何にして堅牢なコードへと改善していくか、具体的な事例を交えて解説します。

また、AI技術を活用したコードレビュー手法にも触れ、PR-AgentやCodeRabbitなど最先端のツールを用いた効果的なコードレビュー戦略も紹介すると同時に、実践的なコードレビューの手法とKotlinでのオブジェクト指向設計の知識を身につけることで、チームの技術力向上につながる具体的な方法論を提供します。

Kotlinの魅力を再発見し、より高品質なコードを目指すためにこのセッションが新たな知見とインスピレーションを提供できるセッションとなれば幸いです。

興味が湧いた方は、是非プロポーザルに⭐️を付けて頂けると嬉しいです!

【Kotlin Fest 2024】
https://fortee.jp/kotlin-fest-2024

【プロポーザル】
https://fortee.jp/kotlin-fest-2024/proposal/8af52f3e-0409-4acb-9615-c2c8a358d034

akkie76

April 02, 2024
Tweet

More Decks by akkie76

Other Decks in Programming

Transcript

  1. #kotlinfest • Akihiko Sato • Mobile & Server Engineer •

    𝕏 : @akkiee76 • Speaker Deck : @akkie76 自己紹介 2
  2. #kotlinfest コードレビューの7つの観点 観点 概要 Design 設計、アーキテクチャ Simplicity 理解容易性、可読性 Naming クラス、メソッド、変数などの命名

    Style コードスタイル Functionality 機能要件の充足 Test テストコードの妥当性 Document コメント、ドキュメントの妥当性 10
  3. #kotlinfest • Presentation ◦ ユーザインターフェイス • Damain ◦ ビジネスロジック •

    Data ◦ API, DBにアクセス アーキテクチャ原則違反例 14
  4. #kotlinfest class UsersViewModel @Inject constructor( private val getUsersRepository: GetUsersRepository, private

    val loadBitmapRepository: LoadBitmapRepository ) : ViewModel() { class UsersViewModel @Inject constructor( private val getUsersRepository: GetUsersRepository, private val loadBitmapRepository: LoadBitmapRepository ) : ViewModel() { class UsersViewModel @Inject constructor( private val getUsersUseCase: GetUsersUseCase ) : ViewModel() { class GetUsersUseCaseImpl @Inject constructor( private val getUsersRepository: GetUsersRepository, private val loadBitmapRepository: LoadBitmapRepository ) : GetUsersUseCase { UseCase を DI 直接 Repository を DI 15
  5. #kotlinfest 単一責任原則違反例 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 つの責務を持つ関数 16
  6. #kotlinfest 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 指摘で責務分離を推奨 17
  7. #kotlinfest 密結合例 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クラスに依 存している 18
  8. #kotlinfest 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 指摘で疎結合を推奨 19
  9. #kotlinfest 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) 密結合例 依存関係を注入し 疎結合へ設計変更を推奨 20
  10. #kotlinfest 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) 完全コンストラクタ Coming Soon 🚀 22
  11. #kotlinfest 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) Value Object Coming Soon 🚀 23
  12. #kotlinfest 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) ストラテジパターン Coming Soon 🚀 24
  13. #kotlinfest 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) ポリシー Coming Soon 🚀 25
  14. #kotlinfest 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) ファーストクラスコレクション Coming Soon 🚀 26
  15. #kotlinfest 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) スプラウトクラス Coming Soon 🚀 27
  16. #kotlinfest 2. Simplicity Simplicity 指摘に該当するケース • 分岐が多い / ネストが深い if

    文 • 複雑な三項演算子文 • stream, filter, map を多用した Object 整形文 • 冗長な SQL など 29
  17. #kotlinfest 分岐が多い 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 でネストの解 消を推奨 30
  18. #kotlinfest 分岐が多い 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 でネストを解消し可 読性アップ! 31
  19. #kotlinfest 複雑な三項演算子例 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); } 直感的に分かりにくい 三項演算子 32 int compare(int a, int b, int c) { if (a > b) { if (a > c) { return a; } } else { if (b > c) { return b; } } return c; }
  20. #kotlinfest 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() 期待値が分かりにくいので 理解容易性を推奨 33
  21. #kotlinfest 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() 理解容易性が向上 34
  22. #kotlinfest パフォーマンス比較例 // 金額をカンマ区切りに変換する 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 43
  23. #kotlinfest 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); } } 閾値の変更があった場合もテストが成功してしまう 😱 46
  24. #kotlinfest 定数がテストクラスに再定義されている例 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 で 同じ定数を 参照するように推奨 47
  25. #kotlinfest 主な AI コードレビューツール • PR-Agent • CodeRabbit • Copilot

    Pull Request 55 • PR-Agent • CodeRabbit • Copilot Pull Request
  26. #kotlinfest PR-Agent とは? Pull Request を AI でレビューを行えるツール • PRの概要作成(

    /describe ) • コードレビュー( /review ) • 改善提案( /improve ) Pull Request 作成時に実行される 🤖 56
  27. #kotlinfest CodeRabbit とは? Pull Request を AI でレビューを行えるツール • PRの概要作成

    • コードレビュー( Line-by-line reviews ) • 対話機能( Chat with CodeRabbit ) PR-Agent と近い機能 🤖 57
  28. #kotlinfest PR-Agent の特徴 • Single GPT-4 call • Effectively tackle

    both short and long PRs • JSON prompting strategy • Multiple git providers ◦ GitHub, Gitlab, Bitbucket • Multiple models ◦ GPT-4, GPT-3.5, Anthropic, Cohere, Llama2 58 https://github.com/Codium-ai/pr-agent?tab=readme-ov-file#why-use-pr-agent
  29. #kotlinfest CodeRabbit の特徴 • Prompt and pattern based customizations •

    Multiple git providers ◦ GitHub, Gitlab • Multiple models ◦ GPT-4, GPT-3.5-turbo( OpenAI ) • Price( subscription ) 59 https://github.com/coderabbitai/ai-pr-reviewer
  30. #kotlinfest PR-Agent を利用するための準備 PR-Agentを利用するためには以下を準備します。 • OpenAI API key(従量課金設定) • GitHub

    personal access token • pr-agent-review.yaml(GitHub Actions) ◦ 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 60
  31. #kotlinfest 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 を有効にする 61
  32. #kotlinfest jobs: pr_agent_job: env: INSTRUCTIONS: >- - 以下の観点で指摘を分類してください - Design(設計観点での指摘)

    - 単一責任の原則違反、密結合、低凝集、DRY原則違反、拡張性などの設計的な問題 - Simplicity(理解容易性、可読性観点の指摘) - ネストが多いif文、複雑な三項演算子、複雑なstream整形文、冗長なSQLなど - Naming(適切な命名判断に関する指摘) - クラス、メソッド、変数など命名が対応する内容に相応しいかどうかの指摘 - Style(コードスタイルに関する指摘) - コードスタイル・コーディング規約違反、静的解析違反、期待権限以上のアクセス修飾子 - Functionality(機能的に不具合に関する指摘) - 不具合、パフォーマンス懸念、不要なパラメータを使用 - Test(テストコードの妥当性に関する指摘) - テストコードカバレッジの保証、閾値の妥当性、テストパラメータ群の適正 - Document(Doc、コメントの妥当性に関する指摘) - 不適切、不正確、JavaDoc・KDocなどのDocは必須 - 上記の指摘観点をCategory欄に記載すること - 上記の指摘観点をprefixに付けること。例えばDesign観点の場合はDesign:とprefixを付けること steps: env: PR_REVIEWER.EXTRA_INSTRUCTIONS: ${{ env.INSTRUCTIONS }} PR_CODE_SUGGESTIONS.EXTRA_INSTRUCTIONS: ${{ env.INSTRUCTIONS }} レビュー観点の プロンプトを追加 62 Coming Soon 🚀 具体的に例示する
  33. #kotlinfest CodeRabbit を利用するための準備 CodeRabbit を利用するためには以下を準備します。 • アカウント作成 / GitHub 連携

    • リポジトリ設定 • レビュー対象 / Prompt 設定 https://github.com/coderabbitai/ai-pr-reviewer?tab=readme-ov-file#disclaimer 63 こちらも利用規約を必ずご確認ください
  34. #kotlinfest var data1: String? = null var data2: String? =

    null var flag: Boolean = false var numericValue: Int = 0 private val value1 = 50 private val value2 = 100 fun process(dataA: String, dataB: String, status: Boolean, value: Int) { data1 = dataA data2 = dataB flag = status numericValue = value send(dataB) val state = if (flag) { if (value >= value1) { if (value == value2) "State1" else "State2" } else { "State3" } } else { "State4" } println("Data $dataA state is $state. Action performed with: $dataB") } private fun send(target: String) { println("Action performed with $target") } 抽象的な命名 変更可能性 ネストの深い 三項演算子 サンプルコードでPRを作成 🛠 66 多重責務関数
  35. #kotlinfest fun process(dataA: String, dataB: String, status: Boolean, value: Int)

    { send(dataB) - val state = if (flag) { + val state = determineState(value) + println("Data $dataA state is $state. Action performed with: $dataB") +} +private fun determineState(value: Int): String { + return if (flag) { if (value >= value1) { if (value == value2) "State1" else "State2" } else { "State3" } } else { "State4" } - println("Data $dataA state is $state. Action performed with: $dataB") } Design: DataProcessorクラスのprocessメソッドは、データの処理だけでなく、状態の決定と出力も 行っています。これにより、単一責任の原則に違反しています。状態の決定ロジックを別の メソッドに分割することをお勧めします。 70
  36. #kotlinfest -val state = if (flag) { - if (value

    >= value1) { - if (value == value2) "State1" else "State2" - } else { - "State3" - } -} else { - "State4" +val state = when { + flag && value >= value1 && value == value2 -> "State1" + flag && value >= value1 -> "State2" + flag -> "State3" + else -> "State4" } Simplicity: processメソッド内のネストされたif文は理解しにくくなっています。 状態の決定ロジックを単純化し、可読性を向上させることをお勧めします。 71
  37. #kotlinfest -var data1: String? = null -var data2: String? =

    null -var flag: Boolean = false -var numericValue: Int = 0 +var inputDataA: String? = null +var inputDataB: String? = null +var processingFlag: Boolean = false +var numericParameter: Int = 0 Naming: クラス変数data1, data2, flag, numericValueは、それぞれが何を表しているのかが 明確ではありません。より具体的な名前に変更することをお勧めします。 72
  38. #kotlinfest 1-7: Design: DataProcessorクラスのプロパティは、データ処理の状態を保持するために 使用されていますが、このクラスの設計が単一責任の原則に反している可能性がありま す。データの状態管理とデータ処理のロジックを分離することで、クラスの責任をより明確 にし、再利用性とテスト容易性を向上させることができます。 9-26: Functionality: processメソッド内でsendメソッドが呼び出されていますが、sendメ

    ソッドの呼び出し条件がdataBの値に依存しているため、特定のデータに対してのみアク ションを実行するという要件がある場合、この設計では対応できません。sendメソッドの呼 び出しを条件分岐内で行うなど、より柔軟に対応できる設計を検討することをお勧めしま す。 75
  39. #kotlinfest AI コードレビューまとめ(所感) 77 PR-Agent CodeRabbit 精度 ◯ ◯ 処理速度

    ◯ △ 安定性 ◯ △* 柔軟性 △ ◯ コスト △(従量課金) ◯(サブスク) * 独自アルゴリズムでレビューが簡易化されることがある Coming Soon 🚀