今日はリファクタリングに泣いた日だったかもしれない。
今日は修正したコードのテストを行っていた。
そこで、エラーは発生しなかったので、コミットしようということになった。
コミットする時は必ず立ち合いが必要となる。
そこで、有識者を呼んで、コードの説明とコミットをお願いしたら、指摘が入ったのだ。
このコード、元あったものの修正なのだが、これが酷い。冗長すぎる作りだったのだ。
そんなことはよくある話、無視して自分の部分の修正だけしようと思ったのだが、自分の修正を入れるために必ずif文を使う必要があったのだ。
しかし、if文を1つ入れるだけで、すでに4つのネストを使っており、5つ目のネストとなることでコーディング規約に違反してしまう。
もちろん、ただ単にクソなコードなだけで、ここまでネストは必要ない。
そこで、俺はコーディング規約に違反しない程度ですむように、軽くリファクタリングしたのだ。
メチャクチャ構造を変えてしまうと、動作の保証が取れないかなとも思ったし、ちょっと面倒だった。
あまりに面倒で、正直触りたくないから、新規クラス作ってはダメか？と聞いたら、新規クラスは開発のコストがかかるから、できればやりたくないということで見送られた。
ただのクラスだったら余裕なのだが、設定パラメータを追加する必要があるクラスというのは確かにコストがかかる。
リリースする時に色々とする作業が増えるからだ。
で、多少リファクタリングをしたコードでレビューを依頼したのだけど、すんなり通り、まぁ自分でも大丈夫だという予想はあったので、そこではあまり気にしなかった。
このクラスは主に入力チェックの役割を果たすクラスだった。
そこで、まったくメンテしてないjUnitのクラスをやはり自分の担当だからということでメンテして、リファクタリングした部分についても問題ないことを確認した。
何で、他人の作った部分のテストもしなければならんのだと思いつつ、テストケースを追加した。
これが後に吉と出るのだが。
そして、結合テストをして、問題なかったので、コミットをお願いしたらダメと言われたのだ。
ダメな理由はリファクタリングが甘いということだった。
ここもっとすっきりするよね？これを聞いて、正直マジかよと思った。
ああ、その通りだ。まだまだリファクタリングできる。
そこで、一旦、分かりました。といってコミッターを席に戻し、構造は変えないまま、出来る限りのリファクタリングを行った。
結果1/3のコードは削れただろう。
いや、本当なら構造自体を変えていいのなら1/2にすることだって出来るクラスだった。
運がいいことに、テストクラスを作っていたので、テストクラスを実行して、軽く動作確認で問題のないことは証明できた。
再度、コミッターを呼んで説明すると、まぁそこまでやればいいだろうということで、コミット出来た。
きっついなー、というのが本音。
俺が作ったところ以外のテストケースを書いて、俺が作ったところ以外をリファクタリングして。
何で、俺が。
やっぱり、他人の作ったコードはロクなことがない。
SIerは免許制にして欲しい。
クズのようなコードしかかけない奴は、俺と関わらないでほしい。
コードの良し悪し、アーキテクチャー的には俺も突っ込みどころ満載だろう、でも、ぱっと見て、そのコード半分にできるよな？って思えるようなコードを書く奴はこの業界から早く退場して欲しい。
急いでる時は余裕ないよ、ほんと。。
リファクタリング全開じゃないとコミット出来ないのか。。。
勉強になった。