とある巨大ロジッククラスを抱えるAPIをリファクタリングした
前提:APIの処理概要
- DBからデータA、Bを取得する
- 分類a,b,cごとに処理が異なる箇所(ロジック①)がある
- 分類a,b,cはAPIのパラメタで特定できる
- データABの値をもとに計算ロジック②、③を行う
- 計算結果をDBに登録する
(本当はループ処理とかたくさんあるけど割愛)
変更前
問題点
- メイン処理がフローとロジックをつめこみすぎている。可読性も最悪
- 全体のフローを追おうとするとAPI→メイン処理といってメイン処理の巨大ソースを上から下まで見ないといけなくなる
- APIとメイン処理の役割分担がちょっと意味不明すぎる
- ロジック①はメイン処理を継承している。APIの最初の方に分類判定(abcのクラス取得)を行っている
解決策
- 呼び出し階層を低層化する
- APIとメイン処理は一緒にしてロジックの呼び出し元を一本にする
- フローはAPIクラスを見るだけでわかるようにする
- 処理のまとまりごとにクラス化する
- ロジックに使うデータ取得とか加工をパラメタ作成クラスとして切り出す
変更後
良くなった点
- フローはAPIクラスを見るだけでわかる
- 処理内容はAPIクラスが呼んでいるそれぞれのクラスを見ればよい
- 実装内容はそれぞれのロジッククラスを見れば良いのでわかりやすい
そもそもどうしてこうなったのか(予想)
- もともとかなり複雑な計算処理が必要で複雑なAPIだった
- 追加要件が発生した時に全体最適を考えずにそのままメイン処理に実装した
- 稼働歴が長くなった結果こうなることはよくある。。。
感想
- リファクタリングたのしいけどあるべきを探すのは難しい
- 自分が最初に見たときに困ったこと、わかりにくかったことをヒントにするとよい
- 私の場合はとにかくフローを追うのが大変だった
- もっとよくできるかもしれないけど誰か教えて
- 少なくともフローは見やすくなったと思う。。
参考文献(今回参考にしたわけじゃないけど)
リーダブルコード―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)
http://amzn.asia/3n7Wss0
新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)
http://amzn.asia/cAMqZdR
シーケンス図について
boostnoteでかきました。便利ですね!