クリーンコードその2の1

記憶する要素が少ないコード

コードを読むときは式や条件を一つ一つ頭の中のスタックに詰め込んでいく事になりますが、一度に頭の中に覚えておける要素って何個くらいなのでしょう。体感では５−６個程が限度かなという気がします。あまり多いと最初に読んだ条件式などがすっぽ抜けて泣く泣く最初から読み直したりする事になるわけです。なので、頭の中に覚えておかなければならない事が少ないコードは良いコードだと思います。

ガード条件を使って例外を速攻切り捨てる

以下は、恣意的な例ですがDVDレンタルのお店で、ある人物がDVDを借りられるかを判断するな処理です。このような書き方だと、ifに該当しなかったデータがそのまま捨てられるのか、またはelse文が出現して別の式によって処理されるのかほぼすべての条件を舐めないと決定できません。今取り扱っているデータと、コード中のほぼすべての式を頭の中でスタックしないと読めないコードであり、最初の１スタックが飛んだりすると一から読み直しになってしまいます。また、メイン処理(この場合会員にDvdを貸し出す処理）が膨大な条件の中に埋もれていて、大事な処理が全く目立たないコードでもあります。

if (aPerson != null) {
    if (aShop != null) {
        if (dvd != null ) {
            if (aShop.isKnowingPersonAsMember(aPerson.getId()) {
                DatingSlip datingSlip = null;
                 if (aDvd.isXrated()) {
                    if(aPerson.isAdult()) {
                        datingSlip = aShop.lend(aPerson, aDvd);
                    } else {
                        throw new PermissionDeniedException();
                    }
                } else {
                     datingSlip = aShop.lend(aPerson, aDvd);
                }
                aShop.addDatingSlip(datingSlip);
            } else {
                throw new PermissionDeniedException();
            }
        } else {
            throw new NoSuchDvdException();
        }
    } else {
        throw new IllegalArgumentException();
    }
} else {
    throw new IllegalArgumentException();
}

この処理はガード条件を使って業務例外ケースをどんどんはじいていく事でかなりシンプルに出来ます。メイン処理に不適格なデータはその場で捨てられるため、そのステップ以降で、その条件について気にする必要がなく頭の中の記憶量もごく少なく済みます。

if (aPerson == null || aShop == null) {
    throw new IllegalArgumentException();
}

if (aDvd == null) {
    throw new NoSuchDvdException();
}

if (aShop.isKnowingPersonAsMember(aPerson.getId()) {
    throw new PermissionDeniedException();
}

if (aDvd.isXrated() && !aPersion.isAdult()) 
    throw new PermissionDeniedException();
}

DatingSlip datingSlip = aShop.lend(aPerson, aDvd);
aShop.addDatingSlip(datingSlip);