クソコードとマイナス生産性の関係

投稿日: 2019/08/16 作成者: masuda

定期的に発生する「クソコード」発言問題だけど、新人の未熟なコードよりも（自称）熟練者のクソコードのほうが害が大きいので、さりげなく「クソコード」というようにしている。例えば、

  • こんなクソみたいなコードを残してしまうと、未来に禍根を残すし
  • 使う身になってみれば、クソみたいなAPIを公開されても誰も使わないし
  • こんなクソみたいなUIは誰も使わなくなるから、削ったほうがコストが安くなりますよ

とかとか。以前は汚物用語は使わないようにしていたのだけど、なぜかインパクトが薄く（自称）熟練者には届かないようなので、時に散弾銃のように混ぜるようにしている。ちなみに、さりげなく「クソ」を混ぜると、当たらない人には当たらずにスルーされる。本人だけには分かるようなので、顔が少し赤くなるので判別がつく。
また、「クソコード」を言うときは、本人に対して言うのではなく、

  • ディスプレイのコードを示しながら、「ここのクソコードなんですけどね」とか
  • 印刷したコードにアンダーラインっぽい「削除ライン」を引きながら「ここのコードなんですけどね」

とやる。コードを書いた本人が悪いのではなく、悪いコードを修正しない当人が悪いという示唆である。プログラマは職人的な能力が問われるので、日々進化する。だから、数日前に書いたコードは、自分が未熟だったゆえに（ものを知らなかったゆえに）書かれたコードであれるのだから、数日後にものを知ったあとで直せばいいのだ。
だから、直せばOK. 直さないのは何かが「クソ」なのだから。

何を参考書にする？

コードレビューに関しては、この２冊があれば十分である。

「ソフトウェア インスペクション」は、IBMで育ったレビュー方式で、厳密にレビューの仕方が規定されている。ざっくりと集まって、ざっくりと（自称）熟練者が新人に対して評価を下すのではなく、レビューを行う品質管理担当がいて厳密に手順を決めておく。きっちりと行うには、IBM ぐらい大きな組織でないと難しいのだが、レビューする日を別に定めて、レビューを記録しながらやると同じような効果が得られる。
最大のポイントは、レビューをするものが事前にレビューをしておくことだ。レビューをするために長い時間取られるが、それだけの効果があるし、なによりも面と向かって言うのではなく、あらかじめ紙面に書いておくことで感情的にならずに済む。また、面と向かったときには「何を基準にして駄目なのか」を明確に言えるようにしておく必要がある。

「ピアレビュー」は、インスペクションよりも緩く、一般的なレビューよりも丁寧なものを示している。コードを前にしてピンポイントで進んでいくのではなく、頭から最後まで通してコードを読み進め、レビューをしていく。このローラー作戦を遂行することで、コードの品質が「標準的」になるのが、ピアレビューの特徴である。コードが標準的になると、他の人から読みやすくなるし、保守もしやすくなる。改修コードを入れるときにも、既にあるコードになじむように「補修」あるいは「補完」することによって、全体の質を同じに揃えることができる。

マイナス生産者を排除するためレビューを行え

一般的に、コードレビューは

  • コードの品質を上げる
  • 新人が作ったコードの間違いを直す

という目的で行うことが多いのだが、最近の新人プログラマはそれなりにプログラムコードを勉強しているし、30年前とは違ってプログラム言語も豊富になってきたので、極端に不味いコードが出てくることは少なくなっている…と思う。会社に属さなくなって10年以上経つので、ほんとうのところはどうなのか分からないのだが（いやいや、頭でっかちな新人っぽい人はちらほら見るので、それなりの「ぴよぴよコード」はあるとおもうのだけど C#のぴよぴよコードをなんとか使える形にするための2つの方法 | Moonmile Solutions Blog）。
と、その前に（自称）熟練プログラマが陥りがちな「クソコード」のほうが有害である。どんなところが有害かというと、

  • 「クソコード」を書いた人にしか分からない、クソ理論でコーディングされて、誰も理解できない。
  • 「クソコード」を書くために、クソみたいなライブラリを付随されていて保守性が悪い
  • 業務システムの場合、テスト費用、保守費用が別途かかってくるので、それを増大させるのは全て「クソコード」である。

という点があげられる。個人開発をしていたり、ずっとその会社でライブラリを作り続けていたりするのならばいいのだが、ある程度の大き目の業務システムになると、複数名が関わらざるを得ない。なので、良かれと思ってコードを改修していていても、それは「クソコード」になり得るし、実際に「クソコード」となってしまう。
よって、クソコードは「レガシーコード」よりも低位にある。逆に、先に書いた通り、クソコードは直せばふつうのコードになる。だから、「直さないクソ」な人がクソコードを生み出す≒マイナス生産者である、という話になるのだ。

「ピープルウェア」にある通り、マイナス生産者は清く排除するのがよろしい。あるいは隔離だ。その方法いくつか逆引きにも書いたし、どこかで検索すれば出てくるだろう。

自称熟練者のクソコードの例

自分の過去のコードを見れば、いくらでもクソコードが出てくる。なので、私の書いたクソコードを紹介しておこう。




static int hxx_number = 0;
...
int HXX_get_NUMBER() 
{
	return hxx_number ;
}

その昔、UnixにC++がなくてC言語しか使えなかったとき（まだg++はなかった）に、オブジェクト指向風にメソッドぽいものを作ったクソコード。呼び出し側は、HXX_get_NUMBER 関数で値を取るわけだが、いやいや面倒くさいし、中身が何か分からない。中規模程度であれば、hxx_number を直接参照したほうが可読性がよいだろう。
可読性がわるくなるので、コードを引き継いだ人が迷惑するクソコードだ。




struct DATA data ;

for ( int i=0; i<2; i++ ) {
	...
	data.array[i] = 0 ;
}

構造体のデータを0で初期化する例だが、for文で2つまでか使わないのは無駄だ。arrayが可変ならば必要かもしれないけど、そもそも「2」を即値で置いているのが駄目だろう。素直に、array[0] = 0; array[1] = 0 ; で十分だし、C言語だから memset( &data, 0, sizeof(data)) と memset を使ってゼロリセットするほうがよい。




$h = $this->params['data']['Reservation']['CompletionTime']['hour'];
$m = $this->params['data']['Reservation']['CompletionTime']['min'];
if (  $this->params['data']['Reservation']['CompletionTime']['meridian'] == 'pm' ) {
	$h += 12 ;
	if ( $h == 24 ) { $h = 12; }
}
$CompletionTime		= "$h:$m";
$h = $this->params['data']['Reservation']['StartTime']['hour'];
$m = $this->params['data']['Reservation']['StartTime']['min'];
if (  $this->params['data']['Reservation']['StartTime']['meridian'] == 'pm' ) {
	$h += 12 ;
	if ( $h == 24 ) { $h = 12; }
}
// 入庫時刻の補正
if ( $h == 12 && $m == 1 ) { $h = 0; }
$StartTime		= "$h:$m";
$CustomerBilling		= $this->params['data']['Reservation']['CustomerBilling'];
$y = $this->params['data']['Reservation']['AcceptanceDate']['year'];
$m = $this->params['data']['Reservation']['AcceptanceDate']['month'];
$d = $this->params['data']['Reservation']['AcceptanceDate']['day'];
$AcceptanceDate		= "$y-$m-$d";
$Remarks			= $this->params['data']['Reservation']['Remarks'];
$Scene				= $this->params['data']['Reservation']['Scene'];

これは、PHPをよく知らないときに作ったコードなのだが、日時変換が駄目過ぎて泣けてくる。いわゆる「自称熟練者」がプログラム言語の流儀をよく調べずに、ひとつだけの金づちでなんとかしようとした悪い例である。WEB APIのGET呼び出しなので、今ならばJSON形式のGMT表記で一発で変換できるパターンである。実際、ここは結合テストをすり抜けてしまって運用時にバグが発覚したものである。つまり、クソコードが被害を出してしまった好例？だ。

個人開発や少人数開発であれば、どんなトリッキーなコードを書いてもいいけど（自分がわかるならば）、小規模であっても製品にする場合や複数名でコードを弄るときには、できるだけ「平坦なコード」を書くのがよい。それはコードの標準化とか、共通ライブラリの利用とかいう上辺の話だけではなくて、コードの質を揃えて資産化するという意味でだ。本格的にはMDAとかもうちょっと模索が必要なところだけど。

カテゴリー: 開発 パーマリンク