LoginSignup
142

More than 5 years have passed since last update.

レガシーコードのメンテナンス担当になったら新人はどうすればいい

Last updated at Posted at 2014-12-21

ここでは5つの世界における「パッケージ」の中でも「商用Webベースソフトウェア」、主に小・中規模での場合についての私見を述べます。なお、ここにおけるレガシーコードとはテストの無いコードや、不吉な臭いのするコードを指します。

人間の話

以下ではその規模毎に処方箋案を記載していますがその規模に関わらず、レガシーでありながらも苦痛を伴いながらもメンテナンスが行われているコードというものは、基本的には価値を生み出し続けてきた偉大なコードです。一般的にあなたが日曜日に書く最高にcoolなコードの、一万倍(※)以上の価値を会社や社会へ提供してきた偉大な存在であるというコードと作者への敬意は心のどこかに置いておきましょう。

また世の中には「作者とコードを完全に分けて考えられる人」「ある程度分けて考えられる人」「完全に自分の一部だと考えている人」の3種類がいます。つまりレガシーコードを痛烈に非難する時、66%ぐらいの確率で作者への痛烈な非難だと受け取られるという事です。

それはそれとして、どのようなコードも体制による程度の差はあれど経年劣化は避けられないという事、レガシーコードのメンテナンスが人によってはファラリスの雄牛のように正気を失わせる苦痛だという事、もまた現実です。新人がそんな役割を担う必要性があるのかというと会社や部署や状況次第ですが、根本的には組織としての価値観の変容が多くの場合最も重要な事柄です。

小規模プロダクトの場合

土台(アーキテクチャ・フレームワーク)から腐っている

ウンコの上に何を載せてもウンコです。ある程度短い期間で書きなおせる見込みがあるなら、外部仕様を元に(なければ聞き込み調査でもリバースエンジニアリングでも何でも使って発掘して)さっさと書き直しましょう。

リプレースに当たってview(HTMLなど)を大幅に書き換える必要がなければ外部仕様を(システムテスト|E2Eテスト|ファンクショナルテスト)として、フレームワークあるいは言語に依存しない形でコード化した上で実施するとよいでしょう。しかし「全く同じ機能を提供するシステムを再度作る」というのは場合によっては政治的な理由で難しいかもしれません。

土台はまともだがアプリケーションコードが腐っている

最低限のシステムテストを書く

一番最初に最低限のシステムテストを書きましょう。そのサイトのURLを全て抜き出して、各ページが正常に200を返しているか、ソフト404(HTTPステータスコード200のエラーページとか)になっていないか、実行時にwarningが発生したらHTMLにwarningメッセージがそのまま表示されるようなイカした仕組みになっていれば、そのメッセージ文字列がページに含まれていないかをテストしましょう。

URLを全てと言いましたが、例えば47都道府県毎にトップページのある求人サイトなのであれば、先ずは東京だけテストするようにしましょう。最低限のテストとは最低限のテストの事です。

極端な話、これはソースコードもドキュメントも読めなくても書けるテストです。sitemap.xmlがあるのであればそれを参考にしてもよいでしょう。もしSeleniumやらCapybaraやらPhantomJSやらJenkinsやらTravis CIやらのテストツール周辺がよくわからないというのであれば、curlを叩くシェルスクリプトをcrontabで回しましょう。

ただしSingle-page ApplicationやヘヴィにJavaScript依存したサイトの場合、単純にHTMLを返せているかどうかの確認だけでは最低限の死活監視もままならない場合があります。その時はがんばりましょう。

自社(自サイト、自プロダクト)のビジネスモデルを知る

あなたのサイトは何によって利益をあげていますか?問い合わせ課金ですか?掲載課金ですか?サービス利用料課金ですか?どのURLが利益に最も直結しているURLですか?最低限のテストを全てのURLについて書いたら、次はそのURLについてのテストを厚くしましょう。それはあなたの命を救ってくれるかもしれません。

スコープを分割する

リファクタリングといっても色々なリファクタリングがありますが、最も優先するべきなのはスコープの分割です。スコープが適切に分割されていないと、程度によってはコードが読みにくいとかではなく、人類には物理的に読めないコードになります。美しいクラス設計とか以前の話なので、第一段階としてまずは読めるようにしましょう。オブジェクト指向言語であれば、それはメソッド分割と呼ばれます。

人類の脳においてワーキングメモリで同時に記憶しておける数字は最大で7個だという話もあります。真偽のほどは知りませんが仮にそれが正しいとすれば、同一スコープ内で8個以上の変数を人間が短期記憶しなければ読めないプログラムは、人間には正しく読めないという事です。(人類は優秀なのでファジーにだいたい正しくは読める可能性があるし、長期記憶化するぐらい対象のコードを写経すれば読める可能性はある)

OOPがサポートされた言語では通常、そこまで極端にスコープが巨大なコードは生まれにくいはずなのですが、昔同一スコープで推定10,000行という魔物に遭遇した事があるので人生何が起こるかわからない。

この読みにくさ複雑さを間接的・機械的に測る指標として循環的複雑度(CC)というものもあります。色々な言語でCCの計測ツールがあるので利用してみるのもいいでしょう。

再代入不可能な変数、イミュータブルなオブジェクトを使う

再代入の必要ない変数であれば再代入不可能な変数を使いましょう。Scalaであればvarではなくvalを、ECMA Script 6ならvarではなくconstを、Javaであればfinal修飾子を変数に付けましょう。またミュータブルである必要がないオブジェクトであればイミュータブルにしましょう。

その変数やオブジェクトの状態変化を短期記憶したり意識する必要がなくなるので、読む時の脳内メモリ消費量を抑えられます。

正しく命名する

ファイル名であれクラス名であれメソッド名であれ変数名であれ型名であれ名前は重要です。正しい単位で分割され正しい名前が付けられたコード単位は、名前だけ見て中身を読み飛ばしても全体を読む事ができるので、コードを読むのに必要な記憶量が圧縮され、私のような脳の搭載メモリ量が小さな人間でも読めるコードになります。また正しい名前はそれ自体がコメントの役割を果たしリーダビリティの向上に役立ちます。

ディレクトリや名前空間ではなく、ファイルやクラス名にふわっとした名前が付けられている場合、そのクラスが何者なのかを作成者本人もわかってないケースが多いため、見かけたら警戒しましょう。ふわっとした名前かつ大きなクラスをアプリケーションコードで見かけたら、ほぼ危険物だと思ってよいでしょう。

IDEの十分なサポートがあれば名前の変更は非常に楽です。ただしメタプログラミングやリフレクションなどを使っている箇所、ソースコードから分離されている名前(XMLの中とかDSLの中とかDBスキーマの中とか)には注意しましょう。IDEのサポートが受けられないのであれば、テストの加護を信じて置換。

重複した処理を共通化する

正しい単位で分割され、正しい名前が付けられていれば、何と何が重複した処理なのかは自然と炙り出されてきます。静的解析で機械的に抽出する事も選択肢に入ります。DRY大事。

リーダブルコード読めば済む話なので省略。

クラスを分割する

クラスとかオブジェクトのある言語だといいね。

UTを書く

既に目の前にあるテスト無しコードを前にしてTDDの聖言を唱えても歴史は変えられないしテストも増えません。効率は悪いかもしれませんが、本当に必要なテストなら後から書いてもペイします。書きましょう。

通常、リファクタリングと言えばクラスを外から見たときの挙動を変えない内部実装の変更をさすと思うので、UTを書いてからリファクタリングをするのも良いでしょう。むしろその方が正規ルートでしょう。しかしリモデリングとでも呼ぶべきような大規模な変更が必要な局面では、あなたやあなたのチームの裁量でどれぐらいのリスクを許容できるかによって戦略は変わるかもしれません。

privateメソッドのUTよりもクラスの振る舞いに対するUTの方が、UTよりもシステムテストの方が、実装との距離があるので内部の変更によって壊れにくいテストです。ただし外部仕様の変更には無力です。あなたのプロダクトが置かれた状況に応じて、何のテストをいつ厚くするかテスト戦略を組み立てると良いでしょう。

中規模プロダクトの場合

あなたがコレをゼロベースで書き直すなら少なくとも1年以上はかかるな、と思うソレがここにおける中規模開発です。ただし、あなたにとって10ヶ月かかる開発でも別のだれかや、1年後のあなたにとっては1ヶ月で終わる開発かもしれません。スピードはゲームのルールを変えます。

土台から腐っている

ウンコの上に何を載せてもウンコですが、1,000万円以上をかけて法的契約で縛られているわけでもない内製でリプレースをするというのを、いち新人の裁量に任せてやらせてくれる環境は少数でしょう。上位レイヤーの判断を待つか、判断を促しましょう。

英語以上に数字(特に金額)は世界共通語です。営業職が相手であれ経営職が相手であれ具体的な数字は説得力を持ち得ます。ただし、例えば滅多に改修が入らないレガシーな社内システムの場合であるとか、必ずしもROIとあなたの精神衛生とで利害が一致するわけではありません。そういう場合は定量化しにくいけど離職リスクや負の教育効果(レガシーコードやアンセキュアなコードのメンテナンスで生まれ育った新人が糞コードを再生産するようになったり、プログラミングが嫌いになったり)の方面から攻めればいいんでしょうか。ケースバイケースなんじゃないでしょうか。

型のサポートを受けられる言語の採用を検討する

人間にとっても機械にとっても、型情報があってコードのリーダビリティが下がる事は通常ありません。ライタビリティの低下も型推論やIDEによるサポートで軽減する事が出来ます。プロダクトが大きくなればなるほど読まずに捨てるような事はしにくく、リーダビリティの重要性は相対的に高まります。採用に値するかはケースバイケースですが、検討には値するでしょう。

土台はまともだがアプリケーションコードが腐っている

リーダブルコードを読もう。

ボーイスカウトルールを守る

あなたが新人であれ、なかれ、至高のリーダブルコードに一撃で書き直すのは難しいものです。先ずはあなたに出来る範囲で、今よりもマシな状態になるよう心がけるべきです。

ファットコントローラと遭遇したら、まずはcontroller内でprivateメソッドとしてスコープを分割してみましょう。本来あるべき場所にメソッドを移すのはその後で良いのです。同時にそれが出来れば理想ですが、それはより難しい作業になります。リファクタリング(controllerクラス内でのメソッド分割)と機能変更(クラスの再デザイン)を同時にやってはいけません。

ボーイスカウト・ルール Robert C. Martin:『プログラマが知るべき97のこと』

実は何も腐ってなんかなかった

無知と無理解は嫌悪を誘発します。流行の技術や言語、ライブラリが必ずしもあなたのプロダクトに適しているとは限りません。新人のあなたがまだ理解出来ていないだけで、無知なあなたには腐っているように見えたそのコードも、全うな理由によって生まれた、今現在も全うなコードなのかもしれません。

例えば先ずは六ヶ月、メンテナンスを続けて外部仕様についての理解を深めても、それでもそのコードベースが腐っていると思えるのだとしたら、そのコードは本当に腐っているか、もしくは新人であるあなたへの教育体制が腐っているか、あなたの脳味噌が腐っているか、世界が腐っています。

まとめ

薄くて読みやすいリーダブルコードを読もう(読ませよう)

薄くないもの

https://www.amazon.co.jp/dp/4798145149
https://www.amazon.co.jp/dp/4798116831

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
142