オブジェクト指向で類型化するコードレビュー

Transcript

1. 〜 チームで育てるオブジェクト指向 〜 オブジェクト指向で 類型化するコードレビュー Akihiko Sato オブジェクト指向 LT 会 vol.4 #ooltjp
2. 自己紹介 Akihiko Sato / 株式会社ラクス Lead Engineer SaaS 開発 (Backend, Frontend) / Mobile 開発 (iOS, Android) 上流工程、コードレビュー、チームの課題改善など 読書 / コーヒー / HHKB / 縄跳び
3. 今日伝えたいこと コードレビューを通して オブジェクト指向をチームで育てよう！
4. レビューの取り組み 現在、私のチームではレビューガイドラインを明文化して、 レビュアーはガイドラインに従ってコードレビューを行ってます。
5. 私が join した当時の課題 ・実装のセオリーが分からない ・レビューの観点が分からない レビューがしっかり行われていない体制だった モバイル開発経験者が少なく実装のノウハウがない
6. その結果・・・ ・技術力がなかなか伸びない ・定期的にバグが発生する
7. レビュー指摘を類型化できないか？ チームメンバーの弱点を表せないか？ どうやって課題を改善するか ガイドライン（明文）を作って、レビューコメントを類型化しよう！ （ レビュー指摘は財産 ）
8. ガイドライン作成にあたって 基本的なアウトラインを決めよう！ 7 つの観点でレビューコメントを類型化することに
9. 観点について ① 指摘の観点を 7 つに分類 1. Design （設計） 2. Simplicity （理解容易性） 3. Naming （クラス、メソッド、変数名などの命名） 4. Style （コードスタイル）
10. 観点について ② 指摘の観点を 7 つに分類 5. Functionality （機能を充足しているか） 6. Tests （自動テストが適切である） 7. Document （コメント、ドキュメンに関連）
11. 観点について ③ オブジェクト指向の観点で重要な項目 ・ Design （設計） ・ Simplicity （理解容易性） ・ Naming （クラス、メソッド、変数名などの命名） ・ Style （コードスタイル）
12. Design （設計） システムにとって適切な責務、振る舞いになっているか。システムと してアーキテクチャを遵守できているか。また、システム全体として 一貫性ある設計になっているか。 ・関心の分離（≒単一責任の原則）違反 ・密結合 ・低凝集 ・ DRY 原則違反
13. Design （設計） class SummerDiscountManager { lateinit var manager: DiscountManager /** * 商品を追加する */ fun add(product: Product): Boolean { if (product.id < 0) { // バリデーション ① throw IllegalArgumentException() } if (product.name.isEmpty()) { // バリデーション ② throw IllegalArgumentException() } val temp: Int = if (product.canDiscount) { // 条件分岐 ① discountMmanageranager.totalPrice + manager.getDiscountPrice(product.price) } else { manager.totalPrice + product.price } return if (temp < 3000) { // 条件分岐 ② manager.totalPrice = temp manager.discountProducts.add(product) true } else { false } } } 「良いコード / 悪いコードで学ぶ設計入門」より
14. Simplicity （理解容易性） システムとして可読性あるコーディングになっているか。処理ができ るだけシンプルな振る舞いになっているか。 ・ネストが深い if 分 ・複雑な三項演算子文 ・ stream, filter, map を多用した Object 整形文
15. Simplicity （理解容易性） 分岐が多い if 分例 val hitPointRate: float = member.hitPoint / menber.maxHitPoint var currentHealthCondition: HealthCondition = HealthCondition.NONE if (hitPointRate == 0) { currentHealthCondition = HealthCondition.DEAD } else if (hitPointRate < 0.3) { currentHealthCondition = HealthCondition.DANGER } else if (hitPointRate < 0.5) { currentHealthCondition = HealthCondition.CAUTION } else { currentHealthCondition = HealthCondition.FINE } return currentHealthCondition 「良いコード / 悪いコードで学ぶ設計入門」より
16. Simplicity （理解容易性） コメントでの指摘例 val hitPointRate: float = member.hitPoint / menber.maxHitPoint var currentHealthCondition: HealthCondition = HealthCondition.NONE if (hitPointRate == 0) { return currentHealthCondition = HealthCondition.DEAD } if (hitPointRate < 0.3) { return currentHealthCondition = HealthCondition.DANGER } if (hitPointRate < 0.5) { return currentHealthCondition = HealthCondition.CAUTION } return currentHealthCondition = HealthCondition.FINE
17. Naming （クラス、メソッド、変数名などの命名） 変数やクラス、メソッドに責務を意図した明確な名前が付けられて いるか。英語文法に誤りがないか。 typo もこれに含まれる。 ・振る舞いと一致しない変数名、関数名 ・責務と一致しない関数名 ・英文法の誤り
18. Style （コードスタイル） コードスタイル言語仕様に準拠しているか。 ・静的解析違反 ・不適切なアクセス修飾子 ・表記違反（スネーク、キャメルなど） ・公式（ Apple 、 Google 等）が公開しているガイドライン違反
19. 具体的な利用方法 指摘例 MUST （ Design ）：ドメインロジックが Controller クラスに実装されてます。 domain 層の対象 package に新しくクラスを作成して実装を移してください。 コメントの prefix として利用する。
20. [ 発展編 ] prefix を付けたコメントを集計 GitLab API （ Note API ）を利用してコメントを集計します。 具体的な方法は、 ラクス Advent Calendar 2021 （ 12/23 ） 「 GitLab API で Merge Request のコメントを一括取得する方法」 をご覧ください。
21. まとめ レビューガイドラインを明文化すると 　① コードレビューでオブジェクト指向が学べる 　② 苦手分野が明らかになる 　③ スキルアップのためのアクションプランが検討しやすい チームでの導入を検討してみてはいかがでしょうか
22. オンラインイベントや中途採用を積極的に行ってます！ 最後に
23. ご清聴ありがとうございました