リファクタリングの道標：リファクタリングを通じて組織を改善していく

こんにちは。トレタ技術顧問の奥野 (@okunokentaro) です。先日のブログ記事『奥野さんと社員のリファクタリング部屋　-リポジトリ層のディレクトリをどう作る？-』に対して多くの反響をいただき、ありがとうございます。いただいたご意見やご質問の一つひとつが非常に貴重であり、大変感謝しています。

いくつか個別に質問をいただくことがあったのですが、それらの質問を要約すると「リファクタリング後のコードもまだツッコミどころだらけでは？」や「リポジトリ層という名前がまずおかしいのでは？」という二つの点に集約されました。これらの質問はとても理解できます。今回の記事では、具体的な個別の質問への個別の回答は控えさせていただきますが、全体的な方針とテーマについてお伝えした上で、トレタが取り組んでいるリファクタリングは何を目指しているのかというお話をします。

前提と目的

今回のブログ記事『リファクタリング部屋』で取り上げているコードは、実運用中のアプリケーションの実際のコードで、すでに顧客が存在するプロダクトです。このアプリケーションは複数人で開発されましたが、現在は抜けたメンバーもいるため、開発当時の状況が分かりづらい箇所もあります。

また、本アプリケーションの開発中には筆者は別プロダクトのリードエンジニアを務めていたため、本プロジェクトへの参入からまだ日が浅く、山積している課題を目の当たりにしている状況です。すでに運用中であるため、すべての変更は何よりもエンバグを防ぐことが最優先とされ、ただでさえ自動テストが少ない状況で、何をリファクタリングするにしても慎重な検証が求められます。

このような状況が続くと、リファクタリングのひとつひとつにも労力が掛かり、難読なコードは新しく参加するスタッフに対しても良い印象を与えません。機能追加が進まないだけでなく、現状のバグ修正すら困難となったまま新しいスタッフを迎え入れるのは非常に忍びないです。

そのため、少しでも現存のスタッフ、そして今後参加する未知のスタッフのために環境を整えていくことが、今回のリファクタリングの目的です。

積み重ねを重視する

今回のリファクタリングは、増改築のための基礎工事というよりは、ひたすら庭の雑草むしりに近いものです。限られた時間の中で、テストも少なく大胆なリファクタリングを実施したときのエンバグのリスクが大きいことも考慮すると、なるべく確実と思われる手法で、なおかつ短工数で大きな効果を生みたいところです。

TypeScriptでの開発を進めていることから、幸い変数名や関数名の修正、ディレクトリの移動やファイル名の修正などは低リスクで実施できます。短すぎる、あるいは長すぎる名前が多く、主旨が判然としない処理も、名前を見直すだけで見通しがよくなることがあります。

また、テストがあまり作成されていないためテストを増やしていきたい気持ちも強いのですが、依存関係の管理がなされていないことから、モックテストを気軽に追加することすら困難です。

このような手に負えない状況から、低リスク短工数で実施できて効果の大きいリファクタリングを考えたときに、残されたスタッフが読み解けないコードを少しでも読解できるようにする必要があると判断しました。どんな改善も読解と観察から始まりますが、まずその読解が困難であるためです。

よって、TypeScriptのコンパイルエラーが出ないように改名や切り出しを繰り返し、ブラウザ上で動作検証を行うという、地味な流れではありますが長期戦を覚悟した確実な積み重ねを重視します。

なぜリファクタリング後のコードもツッコミどころだらけなのか？

リファクタリングをテーマとした記事を掲載した上でこのような質問をいただくのは申し訳ないところではありますが、いくつか背景を説明したいと思います。

先の節でもお話しした通り、このシリーズでは地味な改善を積み重ねていくことを重視しています。そして、1日のリファクタリング作業に割ける時間も限られており、本アプリケーションの他の機能開発や、他プロジェクトへのフォローなど複数の作業を並行している中での地道な作業となります。記事のベースとなっている武市さんとの1on1リファクタリングタイムも、無制限に1日中実施できるわけではありません。

そのため、「記事にして皆さんに読んでいただくこと」を目的に、ある特定のコードを極端に磨き上げるようなことはしていません。むしろ、泥臭い現場の中途半端さを包み隠さずに届ける気概でいます。今回の例では、リファクタリング後のコードもまだ名前が怪しかったり、そもそもサンプルに登場するオブジェクトのプロパティ構成が理解しにくいままだったりと、記事で触れていない部分のコード断片も多くの「ツッコミどころ」に溢れていたと思います。それは承知しています。

今回の記事を通じて、まずはファイルの細分化を目指し、Repositoryとは何かという既成概念に囚われずに改善に取り組むことを優先しました。その上でJavaScriptの第一級関数を取り扱える特性を活かし、ファイルや処理を細分化することによって、名前を付ける対象を減らし、より可読性と情報量に優れた名前を付け直せるようにすることがテーマでした。

リファクタリングには正解も不正解も存在しないと思っています。その行いが時を経てどう改善につながるか、あるいは逆に悪い方向に向かってしまうか、それだけのことです。今回は積み重ねを重視する上で、悪い方向に滑っていかないようにフォローすることを重視しました。読者によってはリファクタリング後のサンプルコードが「不正解」に映ったかもしれませんが、トレタの現場ではこれを「前進した」と捉えています。

また、一つのコード断片を極限まで磨き上げてしまうと、今回ご紹介していない他の多くのツッコミどころだらけなコードが、すべて取り残されることになってしまいます。そうではなく、アプリケーション内のすべてのコードに対して満遍なく実施しやすく、かつ負担も少ないという「効くリファクタリング」を考えるようにしています。それがファイル分割の指針であったり、関数分割の指針であったりします。お手本のコードを「少しよくする」ことで、他のコードもお手本通りに「少しよくする」ことができ、全体が底上げできたらまた新たなお手本を一つ選び「少しよくする」という、地道な底上げをひたすら繰り返していくことを目指しています。

そもそもリポジトリになっていないのではないか？

ブログについてもうひとつよくいただいた質問が、「リポジトリ層」という表現や、コード中に登場するRepositoryについての取り扱いに対する疑問でした。この質問もとてもよく理解できます。

現状のコードについてリファクタリング案を出す時、他にスタッフがおらず素案もない場合は筆者が手順をすべて考えて実施していますが、今回はトレタ在籍2年の武市さんが主導でした。そのため、リファクタリングの全体的な方針や理想像はまず武市さんが考え、それに基づいて筆者が壁打ちに付き合うという関係性になっています。

ここで技術顧問という立場として、筆者は提案者である武市さんの案を真っ向から否定することを避けています。つまり「リポジトリ層を設けたい」という相談について、リポジトリ層とは呼べないから提案を却下するという流れには意図的にしていません。そして「その提案はリポジトリ層とは呼べない」といった批評もまずはこらえます。

そうではなく、武市さんの現在の経験と知識からリポジトリ層を設けたいと考えたこと自体を尊重し、その判断に至った経緯をヒアリングしました。

それによると、今回はテストがとても少ない上にGraphQLのライブラリとの密結合がひどく、テストを書くこともままならない点を改善したい。そのためにディレクトリを新規に作り、そこに細分化したファイルを入れたい、とのことでした。ここで、そのディレクトリにRepositoryと名付けることが最善なのかという議論はあえて行わず、まずディレクトリを分けるという判断を最大限に尊重しました。その上で、ブログ内ではcreateServiceRepository()という関数で巨大なオブジェクトを生成する是非についてのみ触れ、別の細分化を提案しています。

トレタの目指すリファクタリングの好循環とは

武市さんの経験量、知識量と筆者のそれには差がありますが、筆者の知識をもって提案を否定することは簡単です。リポジトリ層を設けるという提案自体を否定して、筆者がすべてのレールを敷くことだって可能です。しかし、今回本当に改善したいのはアプリケーションコード自体ではなく、そのアプリケーションコードの改善を通じて円滑になる組織そのものなのです。

冒頭でも説明した通り、新しいスタッフが参入しやすい環境作りを目的としており、コードの改善はあくまでも組織改善の手段のひとつとして捉えています。コードの改善は通過点であり、目的ではないのです。

そのためには、コードの改善の最大化よりも、長期的な組織の関係性を良好にしていくことが重要と判断しました。そのため、武市さんの提案に対しては否定ではなく、更なる知識や事例の共有によってフォローすることに努めました。リポジトリ層という表現がたとえ最適でなくとも、まずはその案をもとに短工数かつ高利益となる改善を探していく。それが今回のファイル分割や改名の第一歩であり、この姿勢がトレタの目指すリファクタリングの好循環を生むための姿勢です。

語彙の選択は難しいところです。色の濃い語彙ほど、組織を超えた共通認識がすでに世界中で形成されているものですから、あらゆる書籍の中から語彙だけを掻い摘んで採用することが不適切になる場面も存在します。筆者としては、多くのアーキテクチャ解説書籍について読解するときに「真似するために読むのではなく、よそさまの事情と思って読むとよい」と伝えています。そして「よそはああやっている。うちはどうやろう？」を考えるための足掛かりになればよく、書籍を語彙リストとして使ってはならないと説明しました。

正直なところ、「奥野さんがリポジトリ層という名前をスルーして他の部分についてだけ指摘しているのは珍しい」という意見もいただいております。「リポジトリ層」という語彙を記事のタイトルに含めることで読者から指摘が生まれるだろうことも承知していました。筆者の性格をよくご存知だなと思いながらも、今回の記事はあくまでも現場の複数日のやり取りの中の1コマであることをご了承いただけると幸いです。