プログラム悪戦苦闘日記

はてなダイアリーからの移行(遺物)

ヘタレプログラムをやっつけろ

 ※2005/05/24 書きなおしました
 
 仕事柄、と言うわけではないはずだが、どういう訳か自分のところにはプログラムのメンテナンスという仕事がよく来る。そのため他人が作ったプログラムを解析する、ということが多い。
 メンテナンスといっても、単にプログラムの可読性を上げればよいということはなく、「パフォーマンスアップ」であることが多い。つまり速くしろ、と。そのため、どこにボトルネックがあるのかを調べるため、プログラムを読む必要がでてくる。本当は、時間測定だけして、時間が掛かっているところだけ直して終わらせる、というのが理想なのだが、現実はそれだけで済むことはない。プログラムが、それをさせてくれない構造になっている。どういうことか? つまり、プログラムが汚すぎて、一部を修正すれば済む、と言うレベルではないのだ。
 ひどいといっても、あまりピンとこない人が多いようだ。おまえがプログラムをちゃんと読めてないのだろうと。しかし、それを言う前に現物を見てほしい。守秘義務とやらがあるので、具体的なプログラムは公開できないが、これに近いものが Cプログラミング診断室 (http://www.pro.or.jp/~fuji/mybooks/cdiag/) の第2部にある。信じられないかもしれないが、というか自分もこの業界にはいるまで信じていなかったが、このテのプログラムが普通に存在しているのである。

 汚いプログラムを紹介しているサイト/本はいくつかあるが、そのプログラムに対処する方法はあまり述べられていないように思う。今回は、そのようなヘタレプログラムに対処する、自分なりの方法を紹介する。しかし、これはまだ研究段階であり、まだまだ発展途上である。以下に紹介する方法は『万能薬』ではないし、『完全』でもない。しかし、手をつけるためのヒントにはなるのではないだろうか。

ヘタレプログラムを見る前に

 まずは事前準備。用意するものは、

  • 腐ったチューニングするプログラムを印刷する

 もしこれを読んでいる人の中には、プログラムを紙に出すことに抵抗を感じる人がいると思う。そんなもの、お気に入りのテキストエディタ1つあれば十分だ、と。しかし、そんなプライドは捨てたほうがいい。対象は、再起不能な糞プログラムである。あなたの常識の範囲にはないプログラムである。とても常識では太刀打ち出来ない代物なのだ。悪いことは言わない、印刷しとけ。でないと後悔します。

  • 赤ボールペンとマーカー

マーカーは最低でも3色欲しい。5色あれば理想だ。これはもちろん、印刷したプログラムに色を塗るためだ。

紙に出して置きながら、テキストエディタも必要なのである。これは検索機能が充実しているものがいい。正規表現が使えるとベストだ。対象は想像をしえない記述をしてくる。例えば変数名が a とか xxx とかだ。変数名 a なんて、普通の検索機能で抽出するのはつらい。しかし、正規表現はなくても使いやすいエディタであればなんとかなる。自分がいつも使っているエディタはTera Padだ。
 
 さて、準備ができただろうか。これからが本番である。

ヘタレプログラムの罠を回避する

 次は実際にプログラムの解析方法だ。

  • コメントは無視する

 記念すべき一等賞は「コメント」だ。通常、重要なヒントになるはずのコメントが、なぜ、無視しなければならないのか? それは、コメントにうそがあるからである。信じられないことだが、納品済みのプログラムのコメントにうそがあるのだ。
 なぜコメントにうそが入るのか。これは私の推測になるが、最初はちゃんと書いてあったのだろうと思う。しかし、テストをしたり仕様変更が起こったりで、プログラムを修正しまくったが、時間がないとかの理由でコメントは放置されていたのではないかと思う。
 ほかにも、コメントを見ても有益な情報がないから、と言う理由もある。プログラムを見れば一目瞭然のものから、「ドキュメントの機能xxの実装」とかだ。

  • else に対応する ifを探す

 「Cプログラミング診断室」にもあるが、if〜elseがなんと遠いことか。しかもネストが深い。ifの6重ネストなんて、ありえないと思ってました。インデントが半角4マスなのに、横スクロールが必要なんて。とにかくelse に対応するifをすべて結び付けて置くこと。対応関係を、印刷した紙に書き込んでおくこと。またC言語系だと中括弧 { } のインデントがあっていない場合が多々あるので、十分に気を付けるけること。

  • エラー処理を消す

 先の操作でif - else の対応付けができたら、エラー処理と思えるif文は消してしまおう。ペンで塗りつぶすのである、2度と目につかないように。とにかくヘタレプログラムの共通な特徴として、正常処理とエラー処理がシマシマになっている、ということだ。これを塗りつぶすことで、ようやく正常処理が何かが見えてくるだろう。

 これは言語によるが、もしグローバル変数、あるいはそれに匹敵するものがある場合は、グローバル変数が使われているところに色を塗ろう。しかし、グローバル変数がたくさんある場合はカラフルになってしまうので、変数を絞る。テキストエディタの検索機能で、たくさん使われている変数が何かを調べてから、色を付けよう。グローバル変数はどこで値が設定/変更されているかが分からない、一番神経を使うところである。また、C言語系のようにポインタがある言語で、ポインタがグローバル変数になっていたときは要注意である。そのポインタに何がセットされているかが不明である場合が多いからだ。関数によって意味が違っている場合が多い。新手の多態性か、と思ってしまう。グローバル変数を使いまわしているのだ。このようなグローバルなポインタは、MS Excelなどで、関数とそのときポインタが指している変数のテーブルを作ることを勧める。もちろん、関数ごとではなく状態ごと、つまり、今どういう処理をしているかで、ポインタが指している変数が変わっている場合がある(つまり、同じ関数であっても、処理タイミングによって指している変数が違う)。この場合は、関数ではなく状態でテーブルを作ろう。柔軟に対処することが重要である。

  • 状態テーブルをつくる

 先のグローバル変数〜のときにも述べたが、状態テーブルをつくろう。グローバル変数を解析したら、そのなかにきっと状態をを表す変数があるはずだ(ない場合は、ここは飛ばして良い)。この状態を表す変数の値によって、関数の動作やグローバル変数の役割が変わる。新種の有限状態オートマトンか、と思ってしまう。この手のプログラムは次のようにな構造になっている場合が多い(C言語の場合)。

int n; // これがグローバル変数

void func1()
{
    switch(n)
    {
    case 1: ...なんか処理...; n = 2; break;
    case 2: ...なんか処理...; n = 3; break;
    case 3: ...なんか処理...; n = 4; break;
    …以下、状態の数だけcaseがある
    }
    
    func2();
}

void func2()
{
    switch(n)
    {
    case 1: ...なんか処理...; n = 2; break;
    case 2: ...なんか処理...; n = 3; break;
    case 3: ...なんか処理...; n = 4; break;
    …以下、状態の数だけcaseがある
    }
    
    func3();
}

void func3() { ... 以下同様 ... }

 こういう構造になっているために、条件分岐が増えるため if - then - else チェーンが膨れ上がり、状態によって関数の動作がかわるため、何をしている関数かがわからない(=ドキュメントが書けない)、となってしまうのだ。
 
 ここまででは、まだまだ完全系には程遠いが、取っ掛かりはできたのではないかと思う。この先は、そのプログラムのクセによってケースバイケースの対応になる。今後はこのクセの見つけ方や、その対処法方を調べることになるが、残念ながらまだそこまで研究が進んでいない。うまく法則性を見つけられればいいのだが。

リファクタリングが適用できない訳

 もしかしたら、リファクタリングを使えばいい、と思った人もいるのではないだろうか。自分もリファクタリングを知ったとき、これでうまく機械的に処理できる、と思った。しかし、適用できないという事実にぶち当たった。なぜならば、リファクタリングが適用できる条件として、

  • そのプログラムにバグがない
  • メソッドや関数の事前条件や事後条件が明確である
  • (xUnitなどの)テストケースがある。またはドキュメントがある

が必要だからだ。しかし、ヘタレプログラムは以上の3つのうちのどれか、というよりすべて満たしていないのが普通である。理由をひとつずつみてみよう。
1.納品されているプログラムにバグがある
 信じられないことだがバグがあるのである。それも特殊ケースではなく、単にプログラムを追っていっただけで発見できるようなものである。例えば、明らかにif文の条件(ifの数)が足りないものである。これは、本当にテストしたのか、なぜ稼動しているプログラムなのにバグが露呈しないか、と疑問に思うが、単にこの関数が呼び出されるときの引数が、つねに1パターン(固定)であるからであった。
2.メソッドや関数の事前条件や事後条件が不明
 ドキュメントを見れば、とかコメントを見れば分かると思う人は、まだ本当の恐怖をしらない人だ。先にも行った通りコメントはダメだ、うそがある。そのプログラムをよく見てみよう。コメントに書かれている関数の引数の数と、実際の関数の引数の数がちがうでしょ。コメントは役に立たない。ではドキュメントはどうか。こんなヘタレプログラムを作るくらいだから、当然まともな日本語を書くことも出来ず、ドキュメントと称するそのコンピューター上のメモリは、プログラムを単に日本語にコンバートしたものである。プログラムをみて分からなければ、ドキュメントをみてもわからないのである。例↓

プログラム
if( price > 1000 )
    total_price = price - price * 0.1;
else
    total_price = price;

ドキュメント
もし、価格が1000円より多ければ、
  全価格は、価格から価格の10%を引いた値を設定する。
そうでなければ、
  全価格は、価格を設定する

(「価格が1000円を超えたら一割引にする」ではだめなのか?)

3.テストケースがない。テストしたドキュメントがない。
 もちろんテストしたのか、とプログラムを納品した会社を問い詰めれば、したと答えるだろう。しかし、きっとドキュメントは無いというだろう。そして必要なら作りますがどうしますか?と聞いてくるだろう。ここで間違えても「作ってくれ」と答えてはいけない。彼らも何をテストしたか覚えていないのだ。そんな彼らが作るドキュメントは見えている。先と同じプログラムからドキュメントを抽出したものだ(手作業でね)。つまり、こちらが把握している以上の情報は得られないということだ。期待してはいけない。時間を無駄にするだけだ。
 そんなわけで、テストケースが書けない。テストケースが書けないからリファクタリングができない(リファクタリングをした結果が、前と動作が同じであることに自信が持てない)。もう、とにかく、だめなのだ。
 

最後に

 ヘタレプログラムと付き合っていると、常人では思いつかないような大技を多く見かける。しかし、このようなところに注意が行ってしまうと、そこに引っかかって先に進めなくなってしまう。プログラムを読む、ということは、あくまで何の処理をしているかを把握することであるので、あまり些細なところにこだわらないほうがいいだろう。そうしないと、時計が12時を過ぎてしまう。適当に対処して人間らしい生活を確保しよう!

参考URL

定番だが、やっぱりここが有名。初心者がみると嫌な気分になるが、この業界にいるとうなずいてしまうことが多い。メンテナンスを繰り返しパッチを当てまくった結果こうなったから仕方ない、と言う人がいるが、それは違うのである。最初から汚いのだ。