複雑なコードをリファクタリングしました
こんにちは。kubopです。
今回は、noteの機能を一部リファクタしました。
元々チーム内でリファクタリングの機運が高まっていて、自分もガッツリとリファクタリング・リアーキテクトはしたことがなくて
以前からやってみたい！と思っていたのでとても良いチャンスでした。
問題となるコード群
巨大なクラス・メソッド
クラスやメソッド自体に責務が多くなり、必然的にレコード数が膨れ上がってしまう。
巨大な分岐
アーキテクチャを考えた実装をしなければ、if分岐が多くなり可読性の低いコードになってしまう。
if… elseif… elseif..
同様にこちらも、同レイヤの概念や、新しいクラスが出来た時にelseifで繋げざるを得なくなる。
理想となる、リファクタ論
SOLID原則
S: 単一責任の原則
アクターの異なるコードは分割する。
O: オープンクローズドの原則
変更が発生するコードは追加するだけで対応する。
L: リスコフの置換原則
サブクラスはそのスーパークラスと置換可能とする。
インターフェース・アブストラクト・具体クラスも同様。
I: インタフェース分離の原則
使わないインターフェースの実装を強制されるべきではない。
D: 依存性逆転の原則
上位モジュールは下位モジュールに依存してはならない。
実装ではなく、抽象に依存せよ。
デザインパターン
実際にデザインパターンに沿ってリファクタしました。
主にストラテジパターンを利用し、巨大なif分岐に対応しました。
バリデーションなどは返り値が決まっているため、Structなどを利用して擬似的なテンプレートパターンを採用していました。
ストラテジパターン: Strategy Pattern
コマンドパターン: Command Pattern
テンプレートパターン: Template Pattern
ファサードパターン: Facade Pattern
実際に行ったこと
全体の概要を理解する
miroや、図を用いてリファクタ対象のドメインを書き起こしました。
責務・仕様をシーケンス図などを用いて表現。
わからないことがあればここに記載し、共通化やパターンに適用できる部分を探しました。
フェーズ毎に外側だけコミット
ある程度クラスの分割方法、ベースクラス、デザインパターンが決まったら具象クラス作成前に、ディレクトリやインターフェースのみ先にコミットしました。
リファクタリングはレビュー難度が高く、これまでの仕様を一度に完全に理解することは難しく、順を追って行っていく必要がありました。
責務が集中していたクラスの責務分割からはじめる
対象ドメインの、中核となるクラスを責務やフェーズ毎に分割できないかと考えたため、ここからまずは手をつけました。
具体的には、バリデーションを1クラスに抽出してひっぺがしたり、外部APIへの疎通や中核処理後の後処理を別クラスに再構築させるということを案として練っていました。
その際は機能的凝集度を意識しました。(もちろん、その中で利用される変数やメソッドの凝集度も！)
変数名の統一化
ポリモーフィックとなるオブジェクトの変数は変数の揺れが多くありました。
そのため、ユビキタス言語とはならないまでも、そのドメイン内で統一させました。その際はチーム内で対象変数の概念について聞いたり、アイデアを募ったりしました。
再利用可能にする
ある巨大なメソッドの一部に存在し、同じ処理が点在していたため
それらの処理を抽出、ストラテジパターンにより呼び出し側で指定した引数に対応する処理を行うように再利用可能にしました。
いまだ全て置き換えは完了していませんが、土台としてあるだけで同じ処理は再複製されず、新しいコンテンツに対応可能になります。
使用されているかわからないメソッドを検証
利用されていないであろうメソッドの真偽を確認するために、
利用されていれば通知されるようなUtilクラスを作成し仕込みました。(Ruby on Railsではdeprecated無かった…)
↓に近いようなものを作ろうとしたけど、とりあえずは通知のみに。
不必要なメソッドは全体を読みにくくし、クラスの責務を撹乱します。
今後もこのUtilクラスは活躍すると思われます。
責務毎にクラスを分割し、単体テストをより細かく作成
単体テストをシナリオテストとするのではなく、
対象のロジックを検証するためのテストとして論理条件を網羅するように作成しました。
前処理が必要なテストでは、既存ロジックを用いることなく必要となる条件をデータを作成することで後から見てもわかりやすくしました。
この処理にはどんなデータがどのような状態になっているのか、が自明であるテスト、そのテストにのみ必要な情報だけを揃えるようにしました。
全てが出来たら既存テストで置き換えて確認する。
実際に交換可能なクラス・メソッドが出来たら、既存のテストが通るように検証しました。
責務が大きいクラスのテストでは、全く予期しない部分でテストが落ちたりするので、適宜仕様を確認の上修正が必要でした。
🤔
テストは命綱
既存テストがなければ確実にリファクタするのは難しい。
難易度はかなり上がるだろうなと思いました。
ユニットテストがない場合、E2E・Characterization testを用いるしかないと思います。
ドキュメンテーションは懐中電灯
古くてもあるだけで助かるドキュメンテーション。
仕様がわからない時は、前を照らす懐中電灯として役に立ちました。
ちなみにサブプロジェクトとして、私はチームでドキュメントを作成していました。ドキュメントの大切さがリファクタをしてより実感しました。
ドキュメントは初期構想が大事だと思うので、以下を読みテンプレートや階層構造を考えています。
書きやすさ、と誰が書いても同クオリティの読みやすさ、検索性能が鍵になると思います。
ゴールは小さくする
リファクタはいくらでもできてしまうので終わりがありません。
ある程度ゴールは小さく決めた上で実行したほうがよかったな、と反省です。
リファクタは楽しい
リファクタはめちゃくちゃ面白いです。
業務を考えた新機能ももちろん面白いですが、内部構造を考えたり
普段本で読んだことがあるパターン、考えをダイレクトに「試し」、「課題解決」することが出来ます。
いままでなんとなく知っていた知識を「使わざるを得ない」状態になり、頭に入ります。
それでもやっぱり難しい
特に進め方が難しかったです。
リファクタはそのドメインの理解度がかなり重要です。
理解していなければ頓珍漢な修正になってしまったり、
技術的負債をさらに生み出しやすいので怖かったりしました。
今回の反省を活かして、次回以降もリファクタを行いたいと思います。
読んだ本
この記事が気に入ったらサポートをしてみませんか？