ソースリファクタリングのすゝめ
どうも
まだリリースされていないシステムがリリース間近になってきているので現在はソースリファクタリングに力を入れています。
ソースリファクタリングとは、要するに書きっぷりがイケてない部分を修正することです。
そもそもイケてないコードを書いていた(というより放置してた)つもりはないのですが、最近畳み掛けるように関係者からの要望が増えた結果、色々見直しが必要になってきたのが大きいですね。
試しに「何かしらのAPIに接続して、返却されたデータを内部向けに加工する一連のメソッド」を整理してみましょう。
何かしらのAPIに接続して、返却されたデータを内部向けに加工するメソッド(){
リクエストボディつくるっぽくない名前のメソッド()
Authorization: basic ~~~
Content-Type: ~~~
HTTPビルダ作成{
プロキシ使わない?使うならどこ経由する?
POST?GET?
ヘッダセット、ボディセットなど
}
HTTPビルダー実行
HTTP実行
API返却からデータA(タイプがカンマ区切りされた文字列)を抽出
抽出データAをカンマ区切りでスプリット
カンマ区切りされた文字それぞれにアクセスしてタイプ別処理を行う
API返却からデータBを抽出
抽出データBを…
API返却からデータCを抽出
抽出データCを…
加工データをそれぞれマッピング
return 加工されたデータ
}
あえてクソを煮詰めました。こんなコードが見えたら内心で画面に向かって中指立てていいと思います。(実際にやったり、まして人に向けちゃダメですよ)
イケてない点はいろいろありますが、まず、長いですよね。ぱっと見でこのメソッド全体がどんな処理をしているのかが読み取れません。とりあえず処理を分けて見やすくしてみたいですよね。
今回の場合、大まかな流れとしては、リクエストヘッダを作る処理、リクエストボディを作る処理、API接続処理、データ抽出処理、データ加工処理に分けられますね。
何かしらのAPIに接続して、返却されたデータを内部向けに加工するメソッド(){
リクエストヘッダつくるっぽい名前のメソッド()
リクエストボディ作るっぽい名前のメソッド()
POSTでAPI接続するっぽい名前のメソッド(リクエストヘッダ、リクエストボディ、エンドポイントetc…)
データ抽出するっぽい名前のメソッド(API返却データ)
データ加工するっぽい名前のメソッド(抽出データ)
return 加工されたデータ
}
おーきれい。ひとめでわかる。こんなんでいいのかって?・・・このメソッドの表面だけで見ればとりあえずこれでいいのです。
さらに突き詰めるのであれば、今回の例だと、API接続はとにかく汎用性が高いし、抽出データAに対してやってるタイプ別処理なんかも内部仕様によっては使い回されるかもしれない。
タイプ別処理は静的なメソッドにしてユーティリティークラスで管理するとよいし、
API接続処理はクラスレベルで作っておいてメソッドたくさん用意してGET用POST用プロキシ有無などの設定が細かく使えるようになっているとなおよい。
ははあ・・・・・・・
でも、動けばよくね?
いや、よくないです。むしろゲロとクソを継ぎ合わせたようなコードがなまじ正常に動いているほうがタチが悪いです。
ちゃんと動くことが分かっているソースを修正する場合、修正の結果バグが発生するということがあってはなりません。特に、一度リリースされたシステムをリファクタリングする場合は慎重にならざるを得ません。
また、最初から綺麗に書いていれば問題ないというわけでもありません。そもそも、ゲロクソコードでなくとも、様々な要望に応えるうちに共通処理が増えたり、抽象化を図る必要のある部分が現れたりというのは十分にありえます。始めから綺麗に組んでたとしてもリファクタリングからは逃げられないのです。
先ほどは、「リリース後の正常に動いてるコードに触れるのって怖いよね。」みたいなことを言いましたが、追加機能の開発を済ませた上で時間が確保できるのであれば、十分に相談し、テスト計画に含めるなりした上で積極的に行っても良いのではないかと思います。むしろそこは避けちゃいけないとこだと思うんですよね。
最後に、リファクタリングを行うにあたって、いくつか意識しておくと良いことがあって、
・コメントは細かく書くことが望ましい(今回の例では省きましたが)。単純にどこからどこまでが何やってるのか書くだけでも後々整理しやすいのでそれが最低限のライン。共通処理が見出しやすくなる効果も期待できる。また、何をやっているかだけじゃなくてなぜそうしているのかを書いておくと他の開発者に面倒なツッコミ入れられることもなくて良い。
・役割の線引き、細分化を図る。細分化がなされ、役割がはっきりしている処理は再利用性が高い。使いまわせるということはそれだけ全体の記述も減り、デバッグする際もその役割を担ったメソッドを修正するだけでよいので管理が楽。
・もうお気づきかもしれませんが、ソースリファクタリングは突き詰めていくと結局オブジェクト指向にたどり着きます。