入社2ヶ月の新卒Webプログラマが感じた既存コードとの向き合い方 ~うんこの上のうんこを作るべからず~
TL:DR
- 既存コードはどんなコードでも尊さがある
- 影響範囲と変更しづらさ
- 無知な共通化は怖い
- 現状のコードはBetterでありBestではない
弊社のシステム概要
僕が在籍している会社は2011年に設立しており, その頃に作られたシステムを現在も運用している.
そのため, 既存システムは5~6年間ほど実運用されてきた歴史がある.
ここ2週の僕の働き
ここ2週は, 1つの追加機能をしていた.
そして, リードエンジニアについてもらい3回設計をやり直した.
結果として僕が考えたコード(200行位)が, 最終的には1つの関数になった.
その理由は, 既存コードの中に"追加機能に必要なロジックを持つ関数"が揃っていたからである.
そこで, 学んだこと, 感じたことを忘れないよう, 思い出せるようこのエントリに書いていく.
僕の失敗
僕は既存コードの把握が甘く, 自分自身で既存機能を再度開発していた.
加えて, 仕事の範囲とは異なる似た機能を持つ部分に手を出し同じロジックを見るように共通化をしていた.
既存コードとの向き合い方
僕が感じた既存コードが持つ特徴
既存コードはどんなコードでも尊さがある
既存のコードは"既に動いている"という点で実績を持っている.
それは"現時点で問題なく動作するもの"と評価を受けていることを意味している.
リファクタリングの際に注意すべきこと | プログラマが知るべき97のこと (以下きのこ本)を引用する.
既存のコードを出来る限り活かすべきです. いかに醜悪なコードであっても, そのコードはテストやレビューを通っているものなのです. 既存のコードを-特に既にリリースされたシステムのコード-をすべて破棄するというのは それまでの数ヶ月(何年)という時間を捨ててしまうということを意味します. (中略) 仮にコードを新たにゼロから書き直したとすると(中略), 過去の作業で得た知識も無駄になってしまいます.
自分が1文字でも変えてしまった時点でそのコードは"動作が未知"であるものに変わってしまう. 元のコードにテストコードがない場合は悲惨で, 本当に動作が担保されない"何か"を生み出すことになる.
自分の場合, 既存コードに存在しているロジックを再度自身の手で再開発していた.
既存コードの把握が甘かったという背景はあれど, 非常に無駄な時間を過ごした.
また, あまり良くないと感じた既存コードのロジックを変え, 自分なりの変更を加えていた.
これはまさに,きのこ本の個人の好みやエゴを入れてはいけませんに反していた.
この手の変更は, 動作を未知にしているだけで殆どの場合は悪影響しか及ぼさない.
できるだけ仕事と関係ないコードには手を入れない, ということが現時点の技術力では大切だと感じた.
影響範囲と変更しづらさ
変更箇所のコードがどの程度の影響範囲を持つのかを気にすることはとても大切である.
例えば…
関数のインタフェースが変わる => 関数を利用している全ての呼び出し元に修正が必要 引数の型が変わる => 呼び出し元や関数内部の処理に修正が必要 関数内部の処理を変える => 意図しない挙動を生み出す 変数名を変える => タイポや修正忘れによるバグが生まれる(controller -> viewへの変数渡しで起きがち) 関数を消す => どこかで機能そのものを失う可能性がある
このように既に動いているものに対する変更は思いもよらぬ変更範囲を伴う.
変更範囲の大きいコードは, “変更しづらい"コードである.
特に"インタフェースの変更"や"関数の削除"はとても影響範囲が広い.
途端にバグを広めることになるため, 変更部分がどれだけの範囲で影響をおよぼすのか把握することは大切である.
これの対策として, リファクタリング前に対象関数のテストを書くことでプログラムの挙動を担保することができる.
外部委託でいらっしゃっている@t_wadaさんとペアプロさせていただいたときにおっしゃっていた言葉を引用する.
(思い出しながら書いているので表現は本人の意図とずれているかもしれません. あくまでも僕の解釈です)
自分が既存関数を変更するときは変更前に必ずテストを書く. そうすると関数の挙動が担保できる. そして, 変更前にどの程度この関数が"有名人"なのかを調べる. あまりにも多い場所で利用されているようであればできるだけ変更は加えたくない. あまり有名人でない, または全然無名なときは変えてしまってもいい. ただし動作は変わらないようにしたい.
テストファーストでなくてもいいのでできるだけテストは書いた方がいいしリファクタリングの際はテストが必要ですという言葉は今後のプログラマ人生の中でずっと活かしていきたいと思う.
無知な共通化は怖い
上記したように,
今回の仕事で仕事の範囲とは異なる似た機能を持つ部分に手を出し同じロジックを見るように共通化を行っていた.
似たようなロジックの共通化は慎重に行うべきである, と指摘を受けた.
それは, 使用される場面や背景が異なるほど慎重にやらなければいけない.
例えば, controllerとbatch処理を行うコマンドで呼び出されるロジックを共通化したとしよう.
これを行うと, その時点で"たまたま似ている"コードかもしれないのに, 共通化することで依存を生むことになる. どちらかの背景や使われ方を反映させると, どちらかも引きづられるように変更が必要になる.
つまり, “変更しづらい & 変更に弱いコード"を生んでしまい後の運用に手間がかかるようになる. このため, 背景の異なる共通化は慎重に行わなければならない.
現状のコードはBetterでありBestではない
こちらに関しては, エンジニアの先輩方の言葉を借りる.
既存コードは, "作られた時点でとり得る時間, 状態, 事業内容における最適解"でしかない. それが"現状の最適解"とは限らない. だけど僕たちはそれを使って物を作っていかなければならない. 例えば, 今あるコードがクソコードだったとして, うんこの上にうんこを乗せてもそれはうんこにしかならない. だから今の状況に合うように少しずつ変えていかなければならないし, 同じようにクソを生み出してはいけない. 今あるコードが"理想的"であると思ってはいけないよ. あとは, 自分が書いたコードも書いた瞬間から陳腐化が始まることはしっていなきゃいけない. どのコードも'road of the うんこ' だって知りながら書くのも1つ意識を置かなければいけない,
酒の席で聞いた話も混じっているのでご了承ください…mm
実際, 新卒の僕ですら驚いて声が出るようなコードがあったりする.
直そうとすると影響範囲が広く結構大変な上に, テストも無かったりする.
既存のシステムはきっとそういう"過去"を持つものだし, それをあるべき姿にしていくことも自分の仕事なんだろうと考えている.
まとめ
- 既存コードはどんなコードでも尊さがある
- 影響範囲と変更しづらさ
- 無知な共通化は怖い
- 現状のコードはBetterでありBestではない
といくつも書いてきたがきっとまた同じように間違えるのだろうな, と予感している.
ここ2週間でいかに設計が難しいか, いかに物を作らずに問題を解決するかが
容易でないかを知れた気がするし, まだまだ能力として身についていないと思う.
ではどのように自分は進んでいけばいいのだろうか.
ここについては先輩の言葉をお借りしてしめたい.
迷ったときは, 聞けばいい. できるだけ早く. だけど, ただ聞いただけでは自分のものにならない. できるだけ自分で考えてみる. 設計を自分で紙でもテストでも書いてみる. そして, 聞いた時の意見と自分の意見の差分を見てみる. それを繰り返していけば自分の中に落とし込んでいける.
この2週間はまさにそれを経験させていただいた日々だったと思う.
忘れずに今週も頑張ろう.
