テストのないレガシーなRailsアプリをリファクタした話

Rails Developers Meetup 2018: Day 2（ https://techplay.jp/event/655769 ）のランチスポンサーセッションで話した資料です。

For Rails Developers Meetup 2018 : Day 2 (03/25) Track A Lunch Session.
Presented by 株式会社スタディスト開発部.

Transcript

1. テストのない レガシーな Railsアプリを リファクタした話 Rails Developers Meetup 2018 Day 2 (03/25) トラック A ランチセッション 株式会社スタディスト
2. 1. スピーカー紹介 2. 開発の背景 3. とりくんだこと 4. 結果 5. 反省点 6. 今後 CONTENTS 2 前段 後段
3. 1．スピーカー紹介 3
4. 4 Motoki Sakano 所属 営業部 営業戦略Unit 趣味 SEO対策 すきなruby/railsメソッド map
5. 5 Balint Erdos 所属 開発部 趣味 クラフトビール すきなruby/railsメソッド to_proc
6. 2．開発の背景 6
7. 2．開発の背景 • プロダクト ◦ マニュアル作成・共有プラットフォーム ◦ BtoB SaaS ◦ 2013/09リリース ◦ 「伝えることを、もっと簡単に」 7
8. 2．開発の背景 • プロジェクト ◦ Ruby/Railsバージョンアップ ◦ 半年強（2017/06〜2018/01/20） ◦ メンバー（3 → 5人） ▪ 詳細な仕様を知らない ▪ 知見も少ない • システム更改 • プロマネ • ruby経験豊富でない 8
9. 2．開発の背景 なぜリファクタに踏み切った？ ↓ 既存のソースコードのままでは 限られた時間内に効率的で漏れのない バージョンアップ対応は 不可能 9
10. 2．開発の背景 • 既存のソースコード ◦ ドキュメントなし＆記憶だより （の割にコードがきたない） ◦ Fat Controller ◦ コピペの嵐 ▪ 3行上に同じコードが書いてあったり ◦ rubocop導入も静的解析等全OFF ◦ コードレビュー機能せず 10
11. 2．開発の背景 • リファクタで以下を図る ◦ バージョンアップに必要な ▪ 対応箇所削減 ▪ 対応漏れ防止 ◦ 大事な作業に時間を割く ▪ リリース計画 ▪ 機能検証・性能検証 11
12. 3．とりくんだこと 12
13. 3．とりくんだこと 実装面 １）既存コードを3分類 ２）モデルに寄せる（あたりまえだけど） ３）共通化（あたりまえだけど） 環境整備面 ４）上記を支えるコード品質向上施策 13
14. 3．とりくんだこと 実装面 １）既存コードを3分類 ２）モデルに寄せる（あたりまえだけど） ３）共通化（あたりまえだけど） 環境整備面 ４）上記を支えるコード品質向上施策 14
15. 3．とりくんだこと １）既存コードを3分類 ① 必要かつ仕様が明確な処理 ② 不要な処理 ③ わからない処理 15
16. 3．とりくんだこと １）既存コードを3分類 ① 必要かつ仕様が明確な処理 → 書き直す • 既存コードに引っ張られない • あるべき流れを書き下す • 書いた流れにそって実装 16
17. 3．とりくんだこと １）既存コードを3分類 ② 不要な処理 → 削除 • 呼び出し元・Viewを確認 • 容赦なく削除（死んだコードも） ◦ コメントアウト ◦ メソッド1行目の return true など... 17
18. 3．とりくんだこと １）既存コードを3分類 ③ わからない処理 → まとめる • 本流とは別の場所に • 消せるとわかったときに消しやすい 18
19. 3．とりくんだこと 実装面 １）既存コードを3分類 ２）モデルに寄せる（あたりまえだけど） ３）共通化（あたりまえだけど） 環境整備面 ４）上記を支えるコード品質向上施策 19
20. 3．とりくんだこと ２）モデルに寄せる • テストが書きやすくなる • 後から直す時に楽 20
21. 3．とりくんだこと 実装面 １）既存コードを3分類 ２）モデルに寄せる（あたりまえだけど） ３）共通化（あたりまえだけど） 環境整備面 ４）上記を支えるコード品質向上施策 21
22. 3．とりくんだこと 3）共通化 ① controller • controller間：concern • APIとWeb間：common ② model • module, service 22
23. 3．とりくんだこと モデル寄せ・共通化の例）キーワード検索 Before：12行のコピペを20ファイル以上で 23 × 20 controller 以上
24. 3．とりくんだこと 24 scope :match_any, (lambda do |keywords| return match_some(:any, keywords) end) scope :match_all, (lambda do |keywords| return match_some(:all, keywords) end) scope :match, ->(keyword) { match_all(keyword) } def self.match_some(anyall, keywords) keywords = all_space_split(keywords) if keywords.is_a?(String) return none if !defined?(self::FIELDS) || !keywords.is_a?(Enumerable) key = anyall == :all ? :and : :or return where(keywords.map { |item| search_conds(item) } .reduce(&key)) end def self.search_conds(keyword) conds = self::FIELDS.map do |field| arel_table[field].matches("%#{keyword}%") end return conds.reduce(&:or) end モデル寄せ・共通化の例）キーワード検索 After：計46行のmodule化（1メソッド4行以下）
25. 3．とりくんだこと 実装面 １）既存コードを3分類 ２）モデルに寄せる（あたりまえだけど） ３）共通化（あたりまえだけど） 環境整備面 ４）上記を支えるコード品質向上施策 25
26. 3．とりくんだこと 4）コード品質向上施策 • Rubocopルール整備・遵守 • コードレビュー • SideCI導入 26
27. 3．とりくんだこと 4）コード品質向上施策 • Rubocopルール整備 ◦ はじめに全部指摘するよう設定 ◦ リファクタをすすめつつ ◦ 開発部内で合意取りながら ▪ インデント ▪ 文字列の「’」「”」統一 ▪ 1行80文字 ▪ ABC指数 27
28. 3．とりくんだこと 4）コード品質向上施策 • Rubocopルール遵守 ◦ 各自のエディタに設定 ◦ ABC指数やメソッド内行数は努力目標 ◦ PR時SideCIで自動チェック 28
29. 3．とりくんだこと 4）コード品質向上施策 • コードレビュー ◦ コードの良し悪しだけじゃなくていい ◦ まず慣れる ◦ わからないことはわかるまで質問 ◦ GithubのReview機能活用 ▪ Approve◯人以上でマージ など 29
30. 4．結果 30
31. 4．結果 • Before → After ◦ stats 31 Before After コード（行） 43695 27753 テスト（行数） 6 3085 コード：テスト 1:0.00 1:0.11 36% 減 500 倍
32. 4．結果 • バージョンアップに必要な対応箇所削減 ◦ 対応漏れ防止も ◦ 本来必要な作業に時間を割けた • 処理の流れが見やすくなった ◦ 真に改善すべき箇所が明確に 32
33. 5．反省点 33
34. 5．反省点 • リファクタ箇所を細分化できたらよかった ◦ まるっと書き直すことになってしまい、 レビューも辛いし、検証も不足がち • 本番環境相当データでの検証が不足 ◦ 機能検証はOKだったが・・・ ▪ 想定していない箇所に大量データ ▪ 不整合データ 34
35. 6．今後 35
36. 6．今後 • 継続的なお掃除 • アタリマエのことをアタリマエに 36
37. スタディストは 「伝えることを、もっと簡単に」 を一緒に実現するエンジニア を募集しています！ 37
38. まずは一緒に ご飯を食べに行きましょう！ コンタクト、お待ちしております！ Wantedly https://www.wantedly.com/companies/studist 公式 http://studist.jp/recruit/ 38
39. “伝えることを、もっと簡単に。” https://studist.jp/