Railsアンチパターン_suzuki_mar_zeals.pdf

Slide 1

Slide 1 text

ZealsのRailsチームで抱えていた問題と対策したことアンチパターンとその解決策テクノロジー開発部鈴木将之 (suzuki_mar)

Slide 2

Slide 2 text

● 自己紹介 ● 今回話す目的 ● アンチパターンと解決策 ● まとめ Agenda

Slide 3

Slide 3 text

自己紹介

Slide 4

Slide 4 text

- StudyPlus - iOSアプリエンジニア - フリーランス - Zealsも社員ではなく、業務委託 - Rails, Ruby - OOP XP(TDD など) DDD Rails プロジェクトコードオーナー suzuki_mar ：suzuki-mar ：suzuki_mar ：：suzuki_mar

Slide 5

Slide 5 text

今回話す目的

Slide 6

Slide 6 text

ZealsのRails プロジェクトで、アーキテクチャレベルでのリファクタをしてきましたそのリファクタしてきた中で特に自分が重要だと思った内容を説明します LTを聞いた人が関わっているプロジェクトのアーキテクチャを考えるきっかけにしてもらえたら幸いです今回話す目的

Slide 7

Slide 7 text

Railsプロジェクトは、去年末ぐらいからアンチパターンに書いてあるよなリファクタを適宜していった現在は、RailsでDDDを上手に活かす方法を模索中今のRailsプロジェクトの状況

Slide 8

Slide 8 text

アンチパターンの説明に以下のプロジェクトを使用しますプロジェクトの種類:TODOアプリ実装している機能:Taskの作成、削除、編集 Taskを完了する　　　　　　　 Taskの予定の調整 Model Task Log TaskGroup User Schedule サンプルプロジェクトの説明

Slide 9

Slide 9 text

1. Serviceクラスの名前を抽象的にする 2. 開発者だけでモデリングする 3. concernの乱用 4. 属人的なコードレビュー説明するアンチパターン

Slide 10

Slide 10 text

アンチパターン1 Serviceクラスの名前を抽象的にする

Slide 11

Slide 11 text

サービスにしたい処理 Taskを作成するときに、TaskGroupなどの複数のレコードをまとめて作成するそのクラス名をTaskServiceにしてしまう Serviceクラスの名前を抽象的にするアンチパターンの例

Slide 12

Slide 12 text

TaskServiceだと問題になってしまう箇所 ● クラス名からなにをするServiceなのかがわかりづらい ● 抽象的なので、どんどんサービスを加えてしまう ○ その結果、クラスが肥大化してしまい、ゴッドクラスになってしまう Serviceクラスの名前を抽象的にするアンチパターンになってしまう説明

Slide 13

Slide 13 text

名前を具体的にする：TaskCreatorなどにする ● クラス名からタスクを作成するということが簡単にわかる ● 具体的なので、当初考えいていたサービスの責務以外を加えない ○ TaskCreatorに、Taskを削除するサービスを追加することは不自然となる Serviceクラスの名前を抽象的にするアンチパターンの改善案

Slide 14

Slide 14 text

アンチパターン2 開発者だけでモデリングする

Slide 15

Slide 15 text

プロダクトオーナーなどに意見を聞かずにRDBを設計する RDBを設計した内容をモデリングとして扱ってしまう開発者だけでモデリングするアンチパターンの例

Slide 16

Slide 16 text

エンジニアとプロダクトオーナーが話すときに、翻訳が必要プロダクトオーナー”Todoを完了したときに、Userのステータスを変更してほしい “ エンジニア “Taskを完了したときにUserのステータスを変更すればいいんですね” (TodoはTaskと翻訳している) 開発者だけでモデリングするアンチパターンになってしまう理由

Slide 17

Slide 17 text

先程の例のような言葉を翻訳する必要が出てくほっておくと後々大きな問題になってしまう ● 新しいエンジニアがチームに加入したとき ○ 翻訳内容を覚えてもらう必要がある ● 仕様などの複雑な会話をするとき ○ 翻訳して話していくのがコストがかかってしまう開発者だけでモデリングするアンチパターンになってしまう理由

Slide 18

Slide 18 text

先に、プロダクトオーナーなどの関係者と、モデル名などの名前を決める先程の例だとTaskにするのか、Todoにするのかその名前をもとに、RDBの設計やクラス設計をする詳しくは、ユビキタス言語を参照開発者だけでモデリングするアンチパターンの解決策

Slide 19

Slide 19 text

● ユビキタス言語をきめたあとも、ユビキタス言語を使用するように保守をする ○ コードレビューなどをする ● ユーザーが別の名前を使用しているるとき ○ プロダクトオーナーと相談をして、名前を変更するかきめる開発者だけでモデリングするアンチパターンの解決策

Slide 20

Slide 20 text

● RDBのテーブル名やカラム名を変更できなくなってしまう ● 名前を変更するときに、変更するコストが莫大になってしまう ○ 技術的負債になってしまう ● 別のサービスにも影響してしまう ○ マイクロサービスの場合開発者だけでモデリングする放置すると

Slide 21

Slide 21 text

アンチパターン3 concernの乱用

Slide 22

Slide 22 text

委譲するコードを書くのがめんどくさいので、 concernを使用する Taskを完了したときと、Scheduleを変更したときにメールを送信する処理を共通化するために concernを使用する concernの乱用アンチパターンの例

Slide 23

Slide 23 text

処理をどこに定義してあるのかがわからりづらい concernでnotifyメソッドを定義する task.notify とかけるだが、Taskを見ただけでは、notifyがどこに定義しているのかがわからない修正するときに、コード検索をするなど手間が必要になり技術的負債になっていs舞う concernの乱用アンチパターンになってしまう例

Slide 24

Slide 24 text

処理を委譲する共通にしたい場合は、Serviceクラスなどに定義するコード例 Notify.send(task) そうすれば、コード例のようにどこに実装しているのかがわかる concernの乱用アンチパターンの解決策

Slide 25

Slide 25 text

concernを使用する場合は、concernを使用する以外に選択肢がないのかを考える処理を共通化したいだけなら、サービスクラスを作成する concernの乱用アンチパターンの解決策

Slide 26

Slide 26 text

アンチパターン4 属人的なコードレビュー

Slide 27

Slide 27 text

コードレビューで判断する項目がないので、レビュアーが各自で判断して、RequestChangeやApprove をする属人的なコードレビューアンチパターンの例

Slide 28

Slide 28 text

Aさんは小さいところ(NITS)でも、RequestChangeにする Bさんは、テストコードを書いていないとかの、確実に RequestChangeにするべきところでも、Approveしているコードレビューで担保したいところを守れていなかったり、必須じゃないところでも変更してしまう属人的なコードレビューアンチパターンになってしまう例

Slide 29

Slide 29 text

コードレビュー時にMustなところを、GitHubのPRのテンプレートに書く(PULL_REQUEST_TEMPLATE.md) レビューイ(レビューしてもらう人)は、PRを出す前に自分でチェックをするレビュアーは、Mustなところをチェックして、対応していなかったらRequestChangeにする属人的なコードレビューアンチパターンの解決策

Slide 30

Slide 30 text

● ユニットテストを書いているか ● N+1がないか ● クラス設計が適切か ○ Serviceにするべき処理をModelに書いていないか ● APIドキュメントを書いているか属人的なコードレビューチェックするべき例

Slide 31

Slide 31 text

チェック項目は、あくまでもリストアップしたものリストアップに含まれていないけど、マージしたらまずいものはあるので、そのときはコメントする必要であれば、RequestChangeにするあまりにも頻繁に発生するものや、今後絶対にマージ指定はいけないものは、チェック項目に追加していく属人的なコードレビューチェック項目じゃないけどコメントしたい場合

Slide 32

Slide 32 text

自動化できるところは自動化する CIなどで、チェックを自動化できるところはチェックを自動化する仕組みづくりをする bulletなどのGemも積極的に利用する人間が確認するところは、目視で確認するクラス設計などは目視で確認したほうが確実属人的なコードレビュー補足

Slide 33

Slide 33 text

まとめ

Slide 34

Slide 34 text

まとめ ● Serviceクラスは具体的な名前をつける ● プロダクトオーナーなどと一緒にモデリングする ● concernを使用しないで共通化できる方法を考える ● コードレビューで確実にチェックする場所は、PRの Templateに定義する

Slide 35

Slide 35 text

まとめ今回のアンチパターンはZealsのRailsプロジェクトで対応したアンチパターンですプロジェクトによっては、今回説明したアンチパターンでも問題ないケースもありますみなさんのプロジェクトでもアンチパターンとなっていそうな部分を共有していただきたいです

Slide 36

Slide 36 text

LET'S START NEXT INDUSTRIAL REVOLUTION WITH OUR FLYING SHUTTLE

Slide 37

Slide 37 text

アピールポイント ● ChatBotの広告(チャットコマース)という新しいドメインモデリングができます ● 社内のユーザーが多く使用している広告管理画面を作成するので、フィードバックをもらえやすいです ○ サービス自体は、日本のChatBot広告システムとして、エンタープライズ市場 NO1なので、大きなクライアントにも使用してもらっている ● RailsでDDDをうまく使っていく方法を模索できる ○ DDDの勉強会や設計共有会を頻繁にしている ○ DDD歴が長いアドバイザーからRailsのDDDについて教えてもらえる

Slide 38

Slide 38 text

Thank you!!