C#リーダブルコード #11_メソッドはできるだけ早く抜ける_返却する値を無駄に変数に入れない

当サイトではアフィリエイト広告を利用しています。

リーダブルコード
Subway Station in Munich, Germany. Train coming in. Strong Motion Blur on Train and People, no recognizable Persons.

今回は,「メソッドはできるだけ早く抜ける」というお話をしていきたいと思います。

enumの追加

今回の解説で使うための次のConditionという名前のenumをObjects.csなどに作成します。どこかのクラスに記述するのではなく,namespaceのReadableの直下に記述していただければOKです。

internal enum Conditon
{
    Setting,
    Start,
    Running,
    Stop,
    Error,
}

BAD:比較が多すぎてNG

先ほど作成したCondition使用して判定するメソッドの例です。

//メソッドはできるだけ早く抜ける
//BAD:比較が多すぎてNG
private bool IsStopping1(Conditon conditon)
{
    if (conditon == Conditon.Setting
        || conditon == Conditon.Stop
        || conditon == Conditon.Error)
    {
        return true;
    }

    return false;
}

この例では,SettingかStopかErrorの時にTrueが返却されるようになっていますが,こういった,複数の判断を一度にする場合は,以前bool型の判断のところで解説した通り,バグの原因になるため,できるだけ1つずつの判断にした方が言いわけです。

BAD:返却するboolを変数に入れて最後にリターンする

複数の判断を1つずつの判断に変更することで,バグが入り込みにくくなりますが,次のように返却するbool型の変数に結果を入れて,最後に返却するような例を見てみましょう。

//BAD:返却するboolを変数に入れて最後にリターンする
private bool IsStopping2(Conditon conditon)
{
    bool result = false;
    if (conditon == Conditon.Setting)
    {
        result = true; //①
    }
    else if (conditon == Conditon.Stop)
    {
        result = true;
    }
    else if (conditon == Conditon.Error)
    {
        result = true;
    }

    //②この辺で条件があってresultの値が変更されているかもしれない


    //BAD:どんな引数が来ても最後まで読まないといけない
    return result; //③
}

この例では,複数の判定を1つずつするように変更されていますが,resultという変数に結果を一度入れて,最後にreturnしています。この場合,バグの調査などでこのメソッドを誰かが読まないといけない場合,例えば①の部分にヒットした場合も,そのあとの処理を読まないといけません。本来はSettingにヒットしたのだから,Trueを返却して終わればいいのですが,resultに入れられているので,②の部分などで,resultの値が変更されていないかを確認したり,③の部分でresultがreturnされていることを確認したりする必要があります。要するに,必ず最後まで読まないといけないメソッドになってしまっているということです。

GOOD:メソッドはできるだけ早く抜ける

前述の通り,返却する値を,result変数に入れる必要はない場合は,できるだけ早く抜けた方が,読む側としては短いスコープで読めるため,より良いコードといえます。

//GOOD:メソッドはできるだけ早く抜ける
private bool IsStopping3(Conditon conditon)
{
    if (conditon == Conditon.Setting)
    {
        //Settingでトレースしていたら,ここまでしか読まなくていい
        return true; //①
    }

    if (conditon == Conditon.Stop)
    {
        return true;
    }

    if (conditon == Conditon.Error)
    {
        return true;
    }

    return false;
}

①のように,SettingにヒットしたらすぐにTrueを返却することで,トレースしているプログラマーからしたら,ここでこのメソッドは読み終わることができます。本当にヒットしなかった時のみ,最後の行まで読むことになり,デバッグ中のプログラマーとしては,大変読みやすくなります。ですので,複数の判定を一度にしない,ということに加えて,ヒットしたら,すぐに抜けるということを意識してください。ただ,ヒットした後に,何かしら値を加工する必要がある場合は,当然result変数などで受けて,加工するなどの処理が必要なのは,念のため補足しておきます。

コードは書くより読む方が多い

プログラマーを始めたころはあまり気が付かないかもしれませんが,プログラムというのは,書くより,読まれる機会の方が,断然に多いのです。書くのは,担当プログラマーが書く1回ですが,読む機会は,デバッグをするプログラマー全員が,バグ修正や,調査などのたびに,何度も読まれる機会があります。ですので,プログラムコードというのは,書く人間の気持ちよりも,読む人間に寄り添った書き方をするべきです。書く人間の仕事はこのメソッドを作ることですが,読む人間は,大量のメソッドを読む中の1つのメソッドがこのメソッドなわけです。だから,1つ1つのメソッドが,短いスコープで読めるように工夫してあげることが,寄り寄りプログラムであり,よりよいプロジェクトであるといえます。