システム開発アンチパターン～旧バージョンのロジックを残すな！

『旧バージョンのロジックを残す』とはどういう事か、まあ大体想像つくと思いますが、一応説明させて頂きますと『新ロジックの側にコメント化して残す』という事です。
僕の会社の例では、以下に示すような定数CONST_Aの値を変更する場合（言語はJava）、

従来のロジックはコメント化し、その上に修正ID・修正者・修正日付等の情報をこれまたコメントで追加し、旧ロジックの下に新ロジックを追加します。


修正前後の差分がソースコードだけで確認できるので一見良さ気です。そのソースコードの修正が絶対に1回までしか発生しないのであれば、これでも良いでしょう。でも現実はそんな訳ありません。長い間メンテして何回も修正されていくと、以下のようにカオス化し、ソースコードが見にくく（醜く）なっていきます。

例ではただの定数宣言部でしたが、計算等を行っているような業務ロジック部分が例のような状態になっていたらもう最悪です。現バージョンのコードが旧ロジックの中に埋もれてしまい、ロジックの流れを追ったりするのに非常に邪魔になります。今年夏に担当した、当エントリを書くきっかけとなったシステムでは、ソースの総行数7000行中旧ロジック3000行という超大作がありましたよ。そのままだと現バージョンのロジック追うのが大変だったので、コメント行の色を深紅のバラ色に変更して追いましたよ。そしたら目がチカチカして堪りませんでしたよ。

今時、Subversion等のバージョン管理ツールという非常に便利な無償ツール使えば、ソースの修正履歴・差分の確認行えるので、旧ロジックは残さず排除すべきです。さらにRedmine等のプロジェクト管理ツール（厳密にはBTS）とバージョン管理ツール連携させれば、修正要件との紐付けもバッチリです。

ちなみに、そのシステムはバージョン管理ツール導入してたんですけどね。2007年にCVSを（遅っ！しかもSVNじゃないのかよ！）。弊害しか生み出さないこんな運用を、何で未だに行っているのか分からない（実際、担当したプログラマは全員が「間違ってる！」と言ってたし）。恐らく『修正IDでgrepして修正箇所を一括確認したい』要するに『楽して確認したい』というのが理由ではないかと僕は予想してました。

4年以上もこんな運用してきて誰も改善してこなかったのが非常に悲しい。いや、改善提案してきたけど通らなかっただけか？とにかく、僕は上司に改善を申し入れました。すると以下のの答えが返ってきました。
「ピンポイントでそのロジックの修正者、修正日付、修正理由を調べたい」
つまり『ソースコードからの修正要件の逆引きがしたい』という事のようです。うーん、結局『楽して確認したい』って事ですよね。ソースコード見るだけで修正要件確認したいと。一々修正台帳やCVSのコミットログからトレースしたくねえよと。そりゃそちら側としては楽で良いかもしれませんが、その為だけにソースコードを醜くし、修正難易度を上げ、それにより修正工数・デグレ発生度アップし、品質低下させるなんて、馬鹿げている。そもそも『ソースコードからの逆引き』なんて、多分バグった時の責任追及時に行うのでしょうけど、そんな機会しょっちゅうあるもんなんですかね？

やっぱり結局は『修正IDでgrepして一括確認』したいのが本音のような気がする。でもこのやり方では修正コメントに修正ID含めるの忘れた場合、確認漏れが発生してしまいます。ていうか実際にありました。しかもその修正が原因でデグレ発生してやがんの･･･。結局、修正箇所の確認は一つ一つバージョン比較して行うしかないと思います。そして確認の際は、前述の様にデグレが発生していないかも確認するため、修正した部分だけでなくメソッド単位で確認すべきだと僕は思います。

ただ、このシステムの場合メソッドの行数が数百～千行だという･･･

システム開発アンチパターン～メソッドのパラメータに配列使うな！

この秋の情報処理技術者試験ですが、「システムアーキテクト」試験を受験します。論文書かなきゃいけないので、ネタ探しにこれまで携わったプロジェクトに関しての個人メモを見返しました。えっと、1年目から愚痴ってるなぁ～（悲）。一部紹介しますと

• 「ながら」仕事をする。音楽聞きながら、メッセンジャーしながら、2ch見ながら・・・。そういうことしながら仕事するギークがいるらしいが、それとは訳が違う。だって低品質だもの。
• 残業時、夕食に出かけて1時間半近く戻ってこない。しかし我が社の勤怠制度では、休憩時間は見なしで固定45分（残業時は＋30分）であり、5時間休憩しても休憩無しでもその時間で固定。当然、1時間以上夕食してても、その間の残業代はきっちり支払われる。ふざけんな！
• 今時（当時2006年）、オープン系システムの開発＆デバッグ端末でメモリ512MBってどういうこと？
• 共通部品で「すべて置換」されたせいでデグレ発生。

...etc。
また殆どのプロジェクトにおいて、エビデンスの見てくれに無駄に手間を掛けさせることへの不満や、他人のソースコードの低品質に対する悪態をついています。今従事しているプロジェクトで苛つかされてるのと全く同じ嘆きもあり。

ちなみに、現在のプロジェクトのシステムは別の会社が作ったものであり、僕の会社作ではないです。余所でも同じ事をしているということは、結構やってしまいがちな事なのかな。であれば、同じ過ちを繰り返させないためにも、過ちによる悲しみを増やさない為にも、ここで一部ご紹介します。アンチパターン（やってはいけないこと）として。

メソッドのパラメータ数が多いからといって、パラメータにHashtableやString[]を使用してはいけない

※これはオブジェクト指向言語に関してです。
商品情報をDBに登録するメソッドがあり、登録値をパラメータとして渡すのですが、それの作りが以下のようになっていました。ちなみに言語はJavaです。

もうね、訳が分かりません。何番目に何が入っているかなんて分かりません。せめてインデックスを定数化しておけと。いや、そう言う問題じゃない。配列で渡しては駄目（ArrayList等も同様）。この他によく使われるのがHashtableなのですが、これも良くない。存在しないキーでget()しても、コンパイルエラーも実行時エラーも発生しないので。
折角オブジェクト指向言語使っているんだから、クラスの形で渡しましょう。これならメソッド名見ただけで何の値か分かる。


return値に関しても同様です。要するに、値の受け渡しにはクラスを使いましょうということです。

＊

以上、超弩級低レベルな話でした。
もう一つどうしても紹介しておきたいことがあるのですが、それはまた別の機会に。