Akio's Log

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

プログラミングの禁じ手Web版 C言語編 - 分量に関するパターン(サルベージ版)

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

プログラミングの禁じ手Web版 C言語編 - 分量に関するパターン
http://web.archive.org/web/20070104215934/www.cmagazine.jp/src/kinjite/c/volume.html

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



(PR)プログラミング作法
(PR)C言語ポインタ完全制覇 (標準プログラマーズライブラリ)
(PR)改訂新版 Cプログラミング診断室

ここから転載です。


ひとつの関数の行数が多すぎる

深刻度:★★★(重度)

[症状]

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

[原因]

多くの場合,怠慢が原因です。すなわち,適当にプログラムを入力する習慣が付いているものと思われます。

[対策/予防]

実はどう対策/予防していいのか筆者にも思い付きません。職業プログラマの場合,1行増やすたびに,いくらか給料をカットするというのはどうでしょうか?

[例外]

試作段階や試行錯誤している場合は許されるかもしれません。しかし,本番ではキチンと関数を分けて整理しましょう。

[備考]

switch文を使った場合,caseラベルが大量にあると,いとも簡単に行数が多くなります。困ったことに最近のウィンドウもののプログラムは巨大なswitch文を簡単に作らせるような仕様になっています。

たとえば,List 25に示したPsprintf関数は85行あります。これを見て,「まだこの程度なら根性を出せば理解できる」という人もいれば,「50行を超えるのは勘弁してくれ」という人もいます。よく聞かれる目安として,「1画面のなかで納まる程度の行数」とか,「プリンタ用紙の1ページ以内に納まるのが限度」などがあります。

List 25

    void Psprintf(char *outBuf,const char *inFormat,...)
    {
        va_list theVA;
        int theI;
        char *theCp;
        char theCb[16];
        static char theN[] = "0123456789ABCDEF";

        va_start(theVA,inFormat);

        while(*inFormat){
            if(*inFormat == '%'){
                switch(*(inFormat+1)){

                case 'c':
                    theI = va_arg(theVA,int);
                    if(isprint(theI)){
                        *outBuf++ = (char)theI;
                    }else{
                        *outBuf++ = '?';
                    }
                    inFormat += 2;
                    break;

                case 'd':
                    theI = va_arg(theVA,int);
                    if(theI < 0){
                        *outBuf++ = '-';
                        theI *= -1;
                    }
                    theCp = theCb;
                    while(theI >= 10){
                        *theCp++ = theN[theI % 10];
                        theI /= 10;
                    }
                    *theCp++ = theN[theI];
                    while(--theCp >= theCb){
                        *outBuf++ = *theCp;
                    }
                    inFormat += 2;
                    break;

                case 's':
                    theCp = va_arg(theVA,char *);
                    while(*theCp){
                        *outBuf++ = *theCp++;
                    }
                    inFormat += 2;
                    break;

                case 'x':
                    theI = va_arg(theVA,int);
                    if(theI < 0){
                        *outBuf++ = '-';
                        theI *= -1;
                    }
                    theCp = theCb;
                    while(theI >= 16){
                        *theCp++ = theN[theI % 16];
                        theI /= 16;
                    }
                    *theCp++ = theN[theI];
                    while(--theCp >= theCb){
                        *outBuf++ = *theCp;
                    }
                    inFormat += 2;
                    break;

                case '%':
                    *outBuf++ = '%';
                    inFormat += 2;
                    break;

                default:
                    *outBuf++ = *inFormat++;
                    break;
                }
            }else{
                *outBuf++ = *inFormat++;
            }
        }
        *outBuf = '\0';

        va_end(theVA);
    }

ところが筆者が実際に目にする例は,そんな「かわいい」話ではなく,2000行とか3000行におよぶ「最長不倒関数」とでもいいたくなる凶悪な例です。ただし,それがめったに現れない特殊な例だと思うと大間違いで,大規模なチームプログラムをやっているところでは,たいして珍しくなかったりします。

こういう巨大関数ができあがる理由はいろいろ考えられますが,ひとつの単純な理由ではなく,複数の理由が絡み合い,積み重なって,できてくる場合がほとんどです。たまに,こういう巨大関数を作るプログラマのなかに「これだけの巨大関数を作れるのは技術的に優れているからだ」と自己満足をしている場合もありますが,見る人が見れば単なる無能である証明だと見破ってしまいます。一刻も早く認識を改めるべきでしょう。

ちなみに先ほどのList 25を複数の関数に分割して書き直すと,List 26のようになります。List 25に比べると,ある程度「構造」が見えやすくなっていることがわかるでしょう。また,このようにすることで,より構造化が進んで,保守性の点でもプログラムの質が向上します。

List 26

    void Psprintf(char *outBuf,const char *inFormat,...)
    {
        va_list theVA;

        va_start(theVA,inFormat);

        while(*inFormat){
            if(*inFormat == '%'){
                switch(*(inFormat+1)){

                case 'c':
                    outBuf = P_Char_Buf(outBuf,va_arg(theVA,int));
                    inFormat += 2;
                    break;

                case 'd':
                    outBuf = P_IntN_Buf(outBuf,va_arg(theVA,int),10);
                    inFormat += 2;
                    break;

                case 's':
                    outBuf = P_Text_Buf(outBuf,va_arg(theVA,char *));
                    inFormat += 2;
                    break;

                case 'x':
                    outBuf = P_IntN_Buf(outBuf,va_arg(theVA,int),16);
                    inFormat += 2;
                    break;

                case '%':
                    *outBuf++ = '%';
                    inFormat += 2;
                    break;

                default:
                    *outBuf++ = *inFormat++;
                    break;
                }
            }else{
                *outBuf++ = *inFormat++;
            }
        }
        *outBuf = '\0';

        va_end(theVA);
    }

    static char *P_Char_Buf(char *ioBuf,int inC)
    {
        if(isprint(inC)){
            *ioBuf++ = (char)inC;
        }else{
            *ioBuf++ = '?';
        }
        return ioBuf;
    }

    static char *P_Text_Buf(char *ioBuf,char *inT)
    {
        while(*inT){
            *ioBuf++ = *inT++;
        }
        return ioBuf;
    }

    static char *P_IntN_Buf(char *ioBuf,int inNum,int inRadix)
    {
        char theBuf[16];
        char *thePtr;
        
        if(inNum < 0){
            *ioBuf++ = '-';
            inNum *= -1;
        }

        thePtr = P_IntN_Buf_Sub(theBuf,inNum,inRadix);
        while(--thePtr >= theBuf){
            *ioBuf++ = *thePtr;
        }
        return ioBuf;
    }

    static char *P_IntN_Buf_Sub(char *ioBuf,int inNum,int inRadix)
    {
        static char theN[] = "0123456789ABCDEF";

        while(inNum >= inRadix){
            *ioBuf++ = theN[inNum % inRadix];
            inNum /= inRadix;
        }
        *ioBuf++ = theN[inNum];
        return ioBuf;
    }


共通部分をまとめない

深刻度:★★(中程度)

[症状]

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

[原因]

不勉強と怠慢が原因です。

[対策/予防]
[例外]

試行錯誤をしている段階やバグ調査などでコードをいじっている状況はかまわないと思います。ただし,本番のときは,きちんと修正しましょう。


「入れ子」のレベルが深すぎる

深刻度:★★★(重度)

[症状]

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

[原因]

怠慢が最大の原因です。処理の流れを考えずに適当にプログラムを入力する習慣が付いていると思われます。

[対策/予防]

必要な処理の流れを再検討して,最適化してからプログラミングし直しましょう。

[例外]

これも,試作段階や試行錯誤している場合は許されるかもしれません。しかし本番ではキチンと整理しておきましょう。

[備考]

前述した「ひとつの関数の行数が多すぎる」の原因のひとつとして,ifやswitchの処理のなかで,さらにif,switchの条件分岐処理を記述する,いわゆる「入れ子」が2重3重になるというケースがあります。もちろんfor,whileのなかで,さらにfor,while,if,switchと「入れ子」にして,どんどん複雑してしまう場合も同様です。いずれにしても,読みにくくて難解なプログラムになるので,フローシートなどを用いて処理の再検討をする必要があります。


構造体のメンバの数が多い

深刻度:★★★(重度)

[症状]

プログラムの解読や改造が困難になります。

[原因]

見積もり,または設計の失敗が原因です。

[対策/予防]

見積もり・設計を慎重にやり直すべきです。

[例外]

本当に必要なメンバが多い場合は仕方ない気もしますが,複数の構造体に分割できないか,あるいはまったく違ったアプローチはないかということを再検討すべきでしょう。

[備考]

以上の解説だけだと,メンバが多いことによる障害の実感がわかないと思うので,List 27に実例をあげておきましょう。空白行も含めて全部で206行もある巨大な構造体ですが,理解できるでしょうか?

List 27

    struct Hoge {
        struct {
            long num;
            struct {
                long a;
                long b;
                long c;
                long d;
            } con;

            double ra;
            char cn[64];
            char vn[64];
            char vc[64];
        } ppp;   

        struct {
            char id[32];
            char vers[32];
        } ctl;

        struct {
            double min;
            double max;
            long beg;
            long end;
        } sb;    

        struct {
            struct {
                long b1;
                long b2;
                long b3;
                long b4;
            } pg;

            struct {
                long gp;
                long im;
                double a1;
                double a2;
                double a3;
                double a4;
                double a5;
            } bra;  

            struct {
                long xm;
                double xmin;
                double xmax;
            } xval;

            struct {
                long p1;
                double pz;
                double pzz;
            } y_range;  

            struct {
                double frp;
                long nred[256];
                long ox;

                struct {
                    long v1;
                    long v2;
                    long v3;
                    long v4;
                    long v5;
                    long v6;
                    long v7;
                } div;

                struct {
                    struct {
                        long l;
                        long w;
                    } smps;
                    long dif;
                    long b0;
                    long b1;
                    long bz;
                } mul;
            } panel;

            struct {
                long fh;
                double lim;

                struct {
                    long y1;
                    long y2;
                    long y3;
                    long y4;
                    long y5;
                } sm;

                long rep;
                long cod[256];
                long th;
            } tht;

            struct {
                long max1;
                long max2;
                long max3;
                
                struct { 
                    double pot[256];
                    double lig[256];
                } plain;
                double liv[256];
            } eps;

            struct {
                long u1;
                long u2;
                long u3;

                struct {
                    struct {
                        double a1;
                        double a2;
                        double a3;
                        double a4;
                        double a5;
                    } av;

                    struct {
                        double b1;
                        double b2;
                        double b3;
                        double b4;
                        double b5;
                    } bv;

                    struct {
                        double c1;
                        double c2;
                        double c3;
                        double c4;
                        double c5;
                    } cv;
                } recv[256];
            } reco;

            struct {
                long t1;
                long t2;
                struct {
                    double tv;
                    double tw;
                } tz;

                struct {
                    long met;
                    double min;
                    double max;
                    double tot;
                } cp[256];
            } cpo;

            struct {
                long ha;
                long hb;
                long hc;
            } hv;
            
            struct {
                long tq;
                long tr;
                long ts;
            } tz;
        } tm[10];

        struct {
            long q1;
            long q2;
            long q3;
            double qz;
        } qit;

        struct {
            struct {
                long a1;
                long a2;
                long a3;
                long a4;
            } av;

            struct {
                long b1;
                long b2;
                long b3;
                long b4;
                long b5;
                long b6;
            } bv;
        } abv;

        struct {
            char gnm[32];
            char gnz[256][32];
            char gpr[32];
        } grp;
    };

ちなみに,この構造体は今回の特集のために筆者がでっちあげたものではなく,実際の業務にあった実例です。もちろん筆者は,「こういうのはあまりにも馬鹿げているのでやめたほうがいい」と進言しました。



構造体の入れ子のレベルが深い

深刻度:★★★(重度)

[症状]

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

[原因]

これも,見積もり・設計の失敗が原因です。

[対策/予防]

見積もり・設計を慎重にやり直すべきです。

[例外]

本当に入れ子が必要な場合はやむをえない場合もあるかもしれません。しかし,プログラムの保守性を考えると,別のアプローチを検討して入れ子が深くなりすぎないようにするべきだと思います。

[備考]

「構造体のメンバの数が多い」パターンで例にしたList 27は,入れ子が深くて複雑になっている例でもあります。

List 27

    struct Hoge {
        struct {
            long num;
            struct {
                long a;
                long b;
                long c;
                long d;
            } con;

            double ra;
            char cn[64];
            char vn[64];
            char vc[64];
        } ppp;   

        struct {
            char id[32];
            char vers[32];
        } ctl;

        struct {
            double min;
            double max;
            long beg;
            long end;
        } sb;    

        struct {
            struct {
                long b1;
                long b2;
                long b3;
                long b4;
            } pg;

            struct {
                long gp;
                long im;
                double a1;
                double a2;
                double a3;
                double a4;
                double a5;
            } bra;  

            struct {
                long xm;
                double xmin;
                double xmax;
            } xval;

            struct {
                long p1;
                double pz;
                double pzz;
            } y_range;  

            struct {
                double frp;
                long nred[256];
                long ox;

                struct {
                    long v1;
                    long v2;
                    long v3;
                    long v4;
                    long v5;
                    long v6;
                    long v7;
                } div;

                struct {
                    struct {
                        long l;
                        long w;
                    } smps;
                    long dif;
                    long b0;
                    long b1;
                    long bz;
                } mul;
            } panel;

            struct {
                long fh;
                double lim;

                struct {
                    long y1;
                    long y2;
                    long y3;
                    long y4;
                    long y5;
                } sm;

                long rep;
                long cod[256];
                long th;
            } tht;

            struct {
                long max1;
                long max2;
                long max3;
                
                struct { 
                    double pot[256];
                    double lig[256];
                } plain;
                double liv[256];
            } eps;

            struct {
                long u1;
                long u2;
                long u3;

                struct {
                    struct {
                        double a1;
                        double a2;
                        double a3;
                        double a4;
                        double a5;
                    } av;

                    struct {
                        double b1;
                        double b2;
                        double b3;
                        double b4;
                        double b5;
                    } bv;

                    struct {
                        double c1;
                        double c2;
                        double c3;
                        double c4;
                        double c5;
                    } cv;
                } recv[256];
            } reco;

            struct {
                long t1;
                long t2;
                struct {
                    double tv;
                    double tw;
                } tz;

                struct {
                    long met;
                    double min;
                    double max;
                    double tot;
                } cp[256];
            } cpo;

            struct {
                long ha;
                long hb;
                long hc;
            } hv;
            
            struct {
                long tq;
                long tr;
                long ts;
            } tz;
        } tm[10];

        struct {
            long q1;
            long q2;
            long q3;
            double qz;
        } qit;

        struct {
            struct {
                long a1;
                long a2;
                long a3;
                long a4;
            } av;

            struct {
                long b1;
                long b2;
                long b3;
                long b4;
                long b5;
                long b6;
            } bv;
        } abv;

        struct {
            char gnm[32];
            char gnz[256][32];
            char gpr[32];
        } grp;
    };

ここまで述べてきた「分量に関する禁じ手」は,わざとギャグでやっているのだろうと思ったら,本人は本当にマジメにやっているケースが多いのですから困ったものです。「プロのプログラマのくせにこんなマヌケな組み方をするのはなぜだ」と思うあなた。その認識は間違っています。むしろ,「プロ」だからこそ,こういうマヌケな組み方が続出するのです。

個人が趣味で開発している場合は,途中でイヤ気がさして挫折するので,それほどひどくはなりません。ところが,プロの仕事には「挫折」は許されません。とにかく仕事をやりとおさなければならないのですから,個人の趣味ならとっくにギブアップしているような汚いコードをえんえんといじらなくてはならないのです。かくして世界中で正気を疑うようなコーディングが続出するわけです。


1ファイル1関数主義にこだわる

深刻度:★★(中程度)

[症状]

「1ファイル1関数」を守ろうとして,ひとつの関数が肥大化してしまいます。その結果,ほかのさまざまな症状を誘発することになります。

[原因]

管理者が楽をしたいと考えているか,あるいは勘違いした「美意識」を持っていることが原因です。

[対策/予防]

妙なスタイルで統一することによるメリットと,それによって発生する弊害を冷静に検討し,改めることです。

[例外]

なし。

[備考]

チームプログラムにはいろんな要員が参加します。こののなかには妙な管理者もいますし,妙な管理テクニックを発揮します。なかには自分の仕事が楽になるように考え,妙な規約を押し付けたり,拘束をしてくる者もいます。

筆者は,初めて「1ファイル1関数主義」を知ったとき,実にナンセンスと感じたのですが,それを提唱した人は大真面目でした。しかも,彼には「善意」まであるのです(勘違いした善意でしかないのですが)。

実際に1ファイル1関数を守ろうとすると,「通常は関数を分割すべきなのだが,別ファイルにしないといけないのがめんどう」という理由で,ひとつの関数が肥大化します。それだけではなく,いままでひとつのファイルに閉じ込められていたグローバル変数の有効範囲が,あいまいになります。

たとえば,「グローバル変数は必要悪だから使用する際にはできるだけその有効範囲を狭くしたい」と考えた場合,List 28のようにstaticを付けて,有効範囲をひとつのファイルのみに限定する方法は常套手段だといえます。このときに,1ファイル1関数主義などを強要されたら,たまったものではありません。せっかく変数の有効範囲を狭めて見通しをよくしようとした努力がすべて無効にされてしまいます。

List 28

    static int gInt; /* こうしておくとファイルの外には有効でない */

    f()
    {
        gInt = 1;
        ...
    }

    g()
    {
        ...
        xxx = gInt;
        ...
    }