Akio's Log

ソフトウェア開発、プロジェクトマネジメント、プログラミング、ランニングなどなど

プログラミングの禁じ手Web版 C言語編 - 記述テクニックに関するパターン(サルベージ版)

C Magazineの「プログラミングの禁じ手Web版 C言語編」を、InternetArchiveからサルベージしました。

プログラミングの禁じ手Web版 C言語編 - 記述テクニックに関するパターン
http://web.archive.org/web/20070203094639/www.cmagazine.jp/src/kinjite/c/coding.html

インデックスはこちら
プログラミングの禁じ手Web版 C言語編(InternetArchiveよりサルベージ) - Akio’s Log

ここから転載です。


インデントを付けない平坦なコーディング

深刻度:★★★(重度)

[症状]

プログラムが著しく読みにくくなるので,解読や改造が困難になります。また,バグが発生しても退治しにくくなります。

[原因]

原因といっても,わざとこんなことをする本人が異常としかいいようがありません。

[対策/予防]

わざとこんなことをするプログラマは,早々にプロジェクトから外すほうがよいでしょう。

[例外]

なし。

[備考]

C言語ではインデントを付けてプログラムを記述するのは常識中の常識というか,こうしないとプログラムの可読性が著しく低下してしまいます。インデントを付けない変人は滅多にいませんが,もしいたとしても注意するのはおそらくムダでしょう。一種の確信犯だからです。


数値を直接コードに埋める

深刻度:★★(中程度)

[症状]

プログラムを変更したいときに,どこを変更すべきか悩むことになります。また,同じ値が数か所で使用されている場合,変更し忘れなどがあるとプログラムの動作がおかしくなります。

[原因]

プログラマの怠慢が原因です。

[対策/予防]

#defineなどを使用して,数値をシンボル化することで簡単に解決できる場合が多いので,そのようなスタイルを勉強して積極的に使用することです。

[例外]

1回こっきりしか使わない値ならコメントを付けて逃げてもよいでしょう。

[備考]

昔から「直値は使用するな」といわれています。たとえば,円周率ならプログラムの式のなかで3.14159という値を直接使うのではなく,PIというわかりやすいシンボルにしてから使うほうがよいとされています(List 29)。ただ,これにも限度があり,たった1回しか使わないシンボルをプログラムの冒頭で大量に#defineしたのではよけいに見にくくなってしまいます。プログラム全体で1回しか使わないような値ならプログラムのコメントでサポートすれば,わざわざ#defineを使うまでもないでしょう。ただし,値がよく変更されるとか,違う値でのテストをしたいケースでは1回しか使わない値にも#defineを使う意味はあります。

List 29

    void f()
    {
        double z,r;
        ...
        z = 3.14159 * r * r;
    }

    #define PI 3.14159

    void g()
    {
        double z,r;
        ...
        z = PI * r * r;
    }


if(XXX == TRUE)

深刻度:★★(中程度)

[症状]

期待したとおりにプログラムが動いてくれないはずです。

[原因]

論理式に対する不勉強とC言語の仕様の理解不足が原因です。

[対策/予防]

C言語の「論理式」について勉強して,正しく使用することです。

[例外]

なし。

[備考]

C 言語特有の落とし穴として「TRUEは1,FALSEは0に決まっている」という思い込みでハマるパターンがあります。たしかに論理式を評価した結果が真なら1,偽は0と明確に規定されていますが,その一方,ifなどで判断される式は,0と同じなら偽,0でないなら真という規定があります。

たとえば,List 30ではIsDiffValueの戻り値は1か0であることが保証されているのでうまくいきますが,IsDiffValueの実装が異なるList 31では期待どおりの結果が得られません。というのもIsDiffValueを作った側は,List 32のような使い方を想定しているからです。

List 30

    int IsDiffValue(int inA,int inB)
    {
        return (inA != inB);
    }

    void f()
    {
        if(IsDiffValue(123,456) == TRUE){
            ...
        }
    }

List 31

    int IsDiffValue(int inA,int inB)
    {
        return (inA - inB);
    }

    void f()
    {
        if(IsDiffValue(123,456) == TRUE){
            ...
        }
    }

List 32

    void f()
    {
        if(IsDiffValue(123,456)) {
            ...
        }
    }

これはC言語に「論理型」というものが明確に定めていないことによる落とし穴で(というか論理型という型はありません),C言語のエキスパートと呼ばれる人たちはif文などでの論理の判断は“1,0"ではなく“非0,0"で行うものだと知っています。さらに,この考えの延長で,「論理値を返す関数は何も“1,0"で返す必要はなく,“非0,0"で返せばいいのだ」となり,これを知らないプログラマとタッグを組んだときに齟齬の元になるわけです。


掛け算の*とポインタ,-と--を判別しにくいようにわざと並べて記述する

深刻度:★★(中程度)

[症状]

プログラムの解読や改造が困難になります。また,期待したとおりにプログラムが動いてくれない場合があります。

[原因]

C 言語ではポインタも掛け算も「*」で表現されるので,「*」と書かれている場合,ポインタなのか掛け算なのかをハッキリ見分けられるように工夫しないと,自分で書いたプログラムでも,わけがわからなくなることがあります。また,同様に「-」と「--」(「+」と「++」)が入り混じると,どこまでが「-」(「+」)で,どこまでが「--」(「++」)なのか悩むことになります(List 33)。

List 33

    f()
    {
        ...
        a = b***c; /* → a = b * (**c) のつもりなんでしょうか? */
        ...
        a = b---c; /* → a = (b--) - c なのか a = b - (--c) なのか紛らわしい */
        ...
    }    

これらをわざとくっ付けて記述して読みにくく記述するプログラマは,読みやすさに配慮するという思慮にかけているか意味のない自慢をしたいのかもしれません。いずれにしても,プログラマの性格に原因があることは間違いありません。

[対策/予防]

紛らわしいと思われるところにはカッコを付けて明示的に区別するように習慣づけることです。演算子の優先順位は規格で決まっていますが,正確に全部を把握している人だけがソースを読むとは限りません。

[例外]

なし。


float,double型の値と数値との比較を==のみで行う

深刻度:★★★(重度)

[症状]

期待したとおりにプログラムが動いてくれないはずです。

[原因]

プログラマが,浮動小数点数には丸め誤差が付きものだと理解していないことが原因です。

[対策/予防]

浮動小数の取り扱いについて,勉強するしかありません。通常は,整数値に変換してから判断するか,比較する数値に誤差を考慮した幅を持たせることで対処します。

[例外]

なし。

[備考]

浮動小数点数を扱う場合,正確に答えがズバリ出るとぴう保証はありません。むしろ,常に「近似値」が出るということを覚悟すべきです。たとえば,List 34のようなコーディングは一見正しいように思えますが,期待したとおりの挙動にならないことがあります。というのも浮動小数点数の演算には「丸め誤差」と呼ばれる誤差が付きものだからです。人間の感覚で10進数で考えたときに割り切れていると思われる式でも,コンピュータの内部の2進数の演算では割り切れずに誤差が発生している場合があります。

List 34

    void f()
    {
        double  a;
        ...
        if(a == 0){ /* ←この部分がまずい */
            ...
        }
        ...
    }

そのため,ある値と等しいかどうかを判断するには,いったん浮動小数点数を整数値に変換してから判断するか(ただし変換するときに桁落としをしないよう注意すること),その値との差の絶対値が,ある程度以下なら等しいとみなすロジックを採用するのが常套手段です。


無節操なgotoの使用

深刻度:★★★(重度)

[症状]

プログラムの解読や改造が困難になります。また,バグが発生しても退治しにくくなります。

[原因]

プログラマが,「goto」の持つ強力な性格を理解していないことが原因です。

[対策/予防]

一切,gotoを使わないか,例外やエラーに関する処理にならマクロやC++の例外処理に切り換えるとよいでしょう。

[例外]

試作段階や試行錯誤している場合は許されるかもしれません。

[備考]

どういうわけか「goto文はよくない」というのはC言語でプログラムをしている人はたいてい知っています。わざわざ「禁じ手パターン」としてピックアップするほどでもありません。しかし,なぜ「よくない」かをきちんと説明できるでしょうか?

理由がわかっていないのに「よくない」といっている人も多いような気がしてなりません。

簡単にいえば,gotoを乱用すると処理の流れが追い切れなくなり,わけがわからなくなるからです。個人でプログラムする場合でも,わけがわからなくなる危険があるのですから,チームプログラムの場合は,なおのことgotoは禁じ手扱いです。

しかし,gotoは,グローバル変数と同様に強力な効能を持っています。また,エラー処理や例外処理に限定して「例外」的にgotoを使う方針もあります。ところが,開発プロジェクトに関わっているすべてのプログラマにそこいらへんがうまく伝わらないと「あ,goto使ってらー,バッカでぇ」と揶揄の対象になったり,あるいは「そうか,gotoを使ってもいいんだ」となって軽率で怠慢なプログラマに都合のいい口実を与えることになりかねません。

ちなみに,List 36のマクロは実際にはgotoを使っているものの,それをマクロのなかに隠すことで安全に「例外処理」を実現した例です。

List 36

    /*
        メモ:
        ここではC++の例外処理機構すなわちtry,catch,throwを
        ある程度、模倣するマクロを定義している。使い方は以下の通り
        
        int theVar; //catchでthrow値を受け止める変数
        
        ptry{           //tryブロックの開始
            ...
            pthrow(値); //throwの実行
            ...
        }
        pcatch(theVar){ //catchブロックの開始
        }

        pcatchで受け止める変数theVarは、あらかじめint型変数として定義すること
        pthrowが実行されなかった場合、theVarには0が無条件に入っている。
        pthrowで投げる値は0以外であること。0である場合、pcatchブロック内は
        実行されない。わざと0を投げるテクニックも使えるかもしれないが、
        C++本来のthrowの仕様通りにはいかなくなる。
    */

    extern int gPTHROW_VALUE__; /* pthrowレベル */

    #define ptry \
        gPTHROW_VALUE__ = 0;

    #define pthrow(VAL) \
        do { \
            gPTHROW_VALUE__ = VAL; \
            goto PCATCH_ENTRY__; \
        } while(0)

    #define pcatch(VAR) \
        PCATCH_ENTRY__:; \
        VAR = gPTHROW_VALUE__; \
        gPTHROW_VALUE__ = 0; \
        if(VAR)

しかし,本当に恐いのは明確に「goto」と表現しているのではなく「隠れたgoto文」になっている場合です。たとえば,breakや continueというのは,gotoと書かれてはいないものの,そこから一気に別の場所に飛ぶという点で,まさしくgoto的な性格を持っています。しかも,gotoと違って,飛び先が明確に書かれていないだけ,よけいにタチが悪いのです。


break,continueの悪用

深刻度:★★★(重度)

[症状]

「無節操なgotoの使用」の場合と同様,プログラムの解読や改造が困難になります。また,バグが発生しても退治しにくくなります。

[原因]

プログラマの怠慢が原因です。適当にプログラムを入力する習慣が付いていると思われます。

[対策/予防]

実はどう対策/予防していいのか筆者にも思い付きません。なにしろ,このようなプログラムを書いている人たちには悪いことをしているという自覚がないのです。

[例外]

試作段階,試行錯誤している場合は許されるかもしれません。

[備考]

List 37のプログラムは筆者がよく見る奇妙な技巧に凝った例を簡略化したものです。ところどころのif文で,どのように処理が飛んでいくのかわかりにくくなっています。break,continueのやっかいなのは,gotoと同様に処理が追いにくくなる危険をはらんでいることに加えて,プログラムの変更でループ文が一段増えただけで,とたんに飛び先が変わる点です。

List 37

    void f(void)
    {
        while(X){
            while(Y){
                g();
                if(Z)
                    break;
                if(A)
                    continue;
                h();
                while(B){
                    i();
                    if(C){
                        j();
                        if(D){
                            continue;
                        }
                        if(E){
                            break;
                        }
                        k();
                    }
                    l();
                    if(F){
                        break;
                    }
                    if(G){
                        continue;
                    }
                }
                m();
            }
            n();
        }
    }

また,break,continueと同様にsetjmp,longjmpを悪用した場合も処理を追いにくくなりプログラムの解読が困難になるので注意が必要です。


キャストの悪用

深刻度:★★★(重度)

[症状]

期待したとおりにプログラムが動いてくれないはずです。

[原因]

プログラマがキャストの意味を理解しないで乱用することが原因です。

[対策/予防]

キャストとはコンパイラに対する黙認指示にすぎず,どのようにコードが生成されるかはプログラマの責任になるということをしっかり認識し,できるだけ使用しないように心掛けることです。

[例外]

キャストをせざるをえない,あるいはキャストを前提とした関数を利用する場合はやむをえないでしょう。しかし,なるべくなら避けるべきです。

[備考]

C言語のダークサイドであり,「グローバル変数」と並んで禁じ手の上級クラスに位置付けられるのが「キャスト」です。

なにしろキャストというのは悪いいい方をすれば「型のごまかし」であり,「コンパイラに対する詐欺行為」にすぎませんから,コンパイラのチェックを通ったものの,やはりうまくいかないというケースは珍しくありません。List 38のプログラムは,その実例です。

List 38

    void f()
    {
        char *theCp;

        theCp = strstr("TEST",(const char *)'S');
        printf("[%s]\n",theCp);
    }

strstrという関数は1番目の引数で指定した文字列から2番目の引数で指定した文字列を探す関数です。List 38では,"TEST"という文字列のなかから'S'という文字を探そうとしているようです。おそらく,

    theCp = strstr("TEST",'S');

と書いてコンパイラに型が一致しないと怒られたのでしょう。「キャスト」で無理やりごまかしています。ところが,strstrで要求している引数は「const char *」,つまりchar型ポインタなのに,実際に与えている'S'はint型です。ここでキャストを使って,「int型だけどcharポインタとして扱ってほしい」とごまかしても残念ながら解決にはなりませんし,思ったとおりには動いてくれません。

このプログラムは正しくは,

    theCp = strstr("TEST","S");

とすべきです。

話は変わりますが,無意味なキャストを避けるために「void *」を使うことがあります。これは,「どこかのアドレスを指すポインタ」という汎用的な意味あいを持たせ,かつ,とくにどの型であるかに束縛させたくない場合に便利です(List 39)。

List 39

    void WriteFD(const void *inData,size_t inSize);

    f()
    {
        int theI;
        char theTxt[128];
        ...
        WriteFD(&theI,sizeof(int));
        WriteFD(theTxt,strlen(theTxt));
        ...
    }

List 39の関数が,もし,「WriteFD(const char * ……)」だったとすれば,呼び出しのときにいちいち「WriteFD((const char *)&theI ……)」というようにキャストを付けて記述をせざるをえませんが,「void *」を使ったため,そういうキャストは不要になります。

ところが世のなかは広いというか,void *の誤った使い方としてList 39をList 40のように記述している例もありました。これは,まったくナンセンスなキャストです。しかも,その人だけではなく同僚の人たちも同じことをしていたのです。会社の「文化」となっている悲惨な例でした。

List 40

    f()
    {
        ...
        WriteFD((const void *)&theI,sizeof(int));
        WriteFD((const void *)theTxt,strlen(theTxt));
        ...
    }


わかりにくい変数名や関数名を使う

深刻度:★★(中程度)

[症状]

プログラムの解読や改造が困難になるので,バグが発生しても退治しにくくなります。

[原因]

プログラマが間違った「美意識」を持っているか,古い時代の制約や常識がいまでもあると勘違いしていることが原因です。

[対策/予防]

今日の現状を勉強して考えを改めることです。それがイヤな老兵にはとっとと消えてもらいましょう。

[例外]

コンパイラやリンカーの制約がある場合は仕方ありません。しかし,現実に多くの処理系が「わかりやすい変数名・関数名」を表現するのに十分な仕様を備えています。

[備考]

プログラムはなるべくわかりやすく作るのがコツです。それは細かい処理部分にまで神経を張りつめてというものではなく,案外,簡単なことを守るだけで改善されるものです。たとえば,わかりやすい名前で関数や変数の名前を付け,スコープ(通用範囲)に応じた命名規則を守るということだけでもずいぶんわかりやすいプログラムになります。

約10年前まではリンカの制約などにより,変数や関数に使用できる名前が6バイトまでという,今日ではナンセンスとも思える環境もありました。そのせいか,古いプログラマのなかには無理やり6バイト以内の名前を付けるのが習慣となって染み込んでいる人もいます。しかし,Windows用プログラムなどでよく見られる非常に長い名前でも問題なく処理できるコンパイラが当たり前になっている今日では,6バイトの制限にこだわった「わかりにくい名前」を使うことはもはや意味のない習慣です。

なかには「K&R第1版の規格では先頭8文字までが有効だから」とか妙な理屈をいう者もいますが,現実に出回っている環境で8文字しか保証していないような処理系は実質ゼロと思っていいでしょう。

名前といえば,ハンガリアン記法という,暗号めいた名前を作り,型情報を名前の一部にするという不思議な記法があります。ある大手のソフト会社の社員が提唱しているので有名ですが,同時に,同じ会社の社員から強烈に批判されていることでも有名な記法です。実をいうと筆者もハンガリアン記法はナンセンスだと思っています。結局,わけのわからない名前になってしまうのと,「この表記を徹底した場合,その変数の型が変更された場合に型情報を含む名前も一斉に変えないといけないのか?」という疑問があるからです。これについては,コンパイラの型のチェックが甘かった時代の遺物ではないかという意見もあります。

筆者は型よりも,むしろスコープ(通用範囲)を基準に命名規則を考えています。これは筆者がよく使うCode WarriorのPowerPlantというクラスライブラリで採用されている命名規則をほとんどそのまま見習っているだけですが,参考までに示しておくと,

  • 関数名は大文字で始め,変数は小文字で始める
  • 外部変数は「g」で始まる名前にする
  • 構造体(あるいはクラス)のメンバ変数は「m」で始まる名前にする
  • 自動変数は「the」で始まる名前にする
  • 引数は参照のみは「in」,書き換えをするのは「out」,両方あるのは「io」で始まる名前にする

あたりが,おもな規則です。こうすることで,どの変数がどれだけのスコープなのかがはっきりわかり,プログラムが読みやすくなるので愛用しています。また,きちんとしたソフト会社プログラマは,この手の命名規則を持っている傾向があるようです。