ひとつの名称を複数の定数で呼ぶなよ……

久々にひどいコードに遭遇したので記録しておきます。

DBの列名を定数にしている

データベースに格納された「製品名」の列を、C#のプログラム中では定数で定義しています。まぁこれ自体は理由があるならば良いと思うのですが、こんな感じです。

const string PRDMST_PRODNAME = "sPrdNameJp";

定数「PRDMST_PRODNAME（製品マスターの製品名）」に、「sPrdNameJp」という列名を定義。DBの列名の命名について知らなくてもPRDMST_PRODNAMEで製品名を指定できるようにしています。他にも製品マスターの列は「PRODCODE」や「UNITPRICE」など、比較的判りやすい定数名で一通り定義されていて、ここを見ればDB定義をしらなくてもある程度列名の指定ができるようになっています。まぁいいでしょう。デメリットもありますが、これを方針として取り入れて徹底するならまぁよし……。

一方で、こんな定義もされています。システム全体の定数だけを宣言したCommonCosntクラスというのがあって、そこで……

const string STOCK_PRODNAME = "sPrdNameJp";

同じ列名に、別の定数名……。嫌な予感がしてきますね。こちらには「在庫マスターの製品名」というコメントが付けられていますが、データベース上では製品名にはすべて「sPrdNameJp」という名称を付けるルールらしく、おなじ文字列が割り当てられています。

1つの処理内で同じものを指すのに違う定数を使う

そしてやらかしているのが、メインのC#プログラム。「製品名」を指定する際に、上記2個の定数の両方が織り交ぜられて使われています。ある処理では「PRODMST_PRODNAME」を。ある処理では「STOCK_PRODNAME」を。どちらも最終的には同じ列名に置き換えられてコンパイルされますから、プログラムの動作上不具合は出ませんし動きます。しかし……ですね。

何が悪いのか

上記の使い方で起こりうる問題を挙げておきますね。他にもあるかもしれませんがとりあえず思いつくものを。

• 列名が変更された場合に一括変更できない …… たとえば製品マスターの製品名のJpが、国際化対応で日本語と限らなくなったので無くなる。名称を「sPrdName」に変更するという地味に大規模な変更が入ったような場合、マスターテーブルの列名はここで定義しているから、とPRDMST_PRODNAMEだけを変更すると、違う定数を使っている箇所が一斉に動かなくなります。まぁデータベースのテーブル列名変更なんてそうそうありませんからこの例ではケアする必要無いかもしれません。ですが、定数の用途の1つとして「1箇所で定義することで集中的に管理できる」というメリットがあるので、これが複数箇所で管理、になった時点でメリットが失せます。
• 変更の影響箇所調査が困難になる …… たとえば数年後に「製品マスターの製品名を参照している箇所はどこかな？」という調査と、それに基づいた改修が必要になったとしましょう。製品マスターの製品名は「sPrdNameJp」。これでソースを検索すると、定数が2つ。製品マスターの処理だから「PRDMST_PRODNAME」で再検索して利用箇所を追跡……とおもったら、製品マスター処理の中に1箇所だけ「STOCK_PRODNAME」を利用していた。なんてことがあると調査漏れの可能性が出てきます。さらに、こういう一貫性のない実装作業が行われていたとなると他の部分も信用できません。結果、調査対象は下記の3通りに増えてしまいます。
  • 製品マスターの製品名に定数PRDMST_PRODNAMEを利用している箇所
  • 製品マスターの製品名に定数STOCK_PRODNAMEを利用している箇所
  • 製品マスターの製品名に定数を使わずsPrdNameJpを直接利用している箇所

プログラムは動くので、初期稼働時には問題は起きません。しかし、数ヶ月、数年経ったところで起きる機能追加や改修、また、利用者からの問合せ対応などでの調査といった保守作業の中で、本来やらなくてよかった作業を行う必要が出てきます。作業量が増えるだけならまぁいいのですが、調査者が気づかなければ見落としや検討漏れといった事態も簡単に発生するので、最終的にコスト、金銭的な部分に影響を及ぼしてしまいます。

SQLアンチパターン

どうすればいいか

これをすれば問題無い、という対策はありませんが、以下のような点に気をつけていくしかないでしょうね。

• 開発、実装の指針を明確にし、浸透させる …… 開発チームが同じ考え方、方針で作業できるような指針を設け、徹底する。
• 開発実装指針に基づいたコードレビューを行う …… 上記のルールに基づいて適宜コードの精査を行い、早期に是正するような工程を入れる。
• 時間があいても、担当組織が変わっても初期の指針を継承させる …… 一番起きやすいのが、初期開発チームの意思を後継者がきちんと継いでいない、というパターンです。特に、開発を外注している場合などは担当企業が変わる可能性があります。その際にもきちんと基本的な考え方を引き継げるような工夫が必要です。

3番目が一番難しいのですが、これを実現するためのポイントは1つ。

• 開発指針はきちんと根拠を添えて提示、資料として残す

ですね。「指針の資料に書いてあるからその通りやる」では、考えや思想は伝わりませんし、気を抜くと抜け漏れが出ます。やはり、引きつぐ人や指示を受けた人が納得できる根拠をきちんと添えておく。これが重要です。たとえば上記のような「列名を定数にする」などの場合でも「なぜそうするのか？」を明確にしておけば、似たようなケースでも開発者は応用を利かせて対応することができますし、「ルールどうなってたっけ？」と判らなくなった際にもいちいち開発指針の資料を読み返さなくても思い出せる確率も上がります。また、結果的に都度の質問が減ったりすることで管理側の手間も省けます。

細かいですが、こういうものを積み重ねた結果が「保守性」に繋がると思うので……。設計書きちんと書いてるから保守性OK！みたいに机上の資料整備だけではなく、プログラミング工程にも眼を向けられるようにする必要がありますね……。

リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)

レガシーコードからの脱却 ―ソフトウェアの寿命を延ばし価値を高める9つのプラクティス