2014年5月28日水曜日

不快もとい深いネストを浅くするためのパターン

仕事中に見かけて,こうすればネストが浅くできてすっきりするのに,と思ったパターンをメモ.

elseの無いif

次のように関数の中身全てがelseの無いifで囲われている場合がある.
void hoge(parameters)
{
    if(condition){
        // やたらと長くてswitchやらifやら混在した処理
    }
}
これは,将来的にelseが追加される可能性があるけれども,現状はこうした方がすっきりする.
void hoge(parameters)
{
    if(!condition){
        return ;
    }
    // やたらと長くてswitchやらifやら混在した処理
}
もちろん,やたらと長い部分も関数に切り分けるなり何なりするべきだけど,とりあえず,条件が満たされなければそれ以降の処理が実行されないのだから,その時点で関数を切り上げれば良い.だいたいこういう書き方をする場合って,ポインタがnullptrの場合に落ちないようにするための安全策だったり,と念のための対処だったりするので,elseが後から追加される可能性も低い気がする.

ifとelseで重複するswitch

void hoge(parameters)
{
    if(condition){
        switch(parameter1){
        case A:
            break;
        case B:
            // 以下延々と続く
        }
    }else{
        switch(parameter1){
        case A:
            break;
        case B:
            break;
            // 以下延々と続く
        }
    }
}
この場合、処理の内容にもよるけれど、少なくとも次のようにifとswitchの関係を入れ替えた方が楽な気がする.
void hoge(parameters)
{
    switch(parameter1){
    case A:
        if(condition){
        }else{
        }
        break;
    case B:
        if(condition){
        }else{
        }
        break;
        // 以下延々と続く
    }
}
これだと、switch文へのcaseの追加は1度で済む.最初の形だと、両方に追加する必要があるから、caseが増えるにつれ、追加忘れが発生する可能性が高くなる.そもそも、そんな長い関数を作るな、と言われそうだけど.あとは、各caseの内容を関数に切り分けるのも良い.
処理の内容次第では、テーブルにデータを入れておいて、caseで必要なテーブルにアクセスする、という形にすれば更にすっきりする.
void hoge(parameters)
{
    // データを引っ張ってくるためのテーブル
    unsigned table[CASE_NUM] = { ... };

    // 実引数が想定外の場合の対応
    if(parameter1 >= CASE_NUM)
        return ;

    unsigned value = table[parameter1];
}
他にも何かあった気がするので、思い出したらメモしていこう.そのうち後輩とかに教えるのに役立つかも.