ケントベック「Smalltalkベストプラクティスパターン」のガード節とアーリーリターン

ケントベック「Smalltalkベストプラクティスパターン」のガード節とアーリーリターンは、とてもシンプルで読みやすくなるコード例です。それに比べると「リファクタリング」や「現場で役立つシステム設計の原則」のアーリーリターンのコード例は、ケントベックのガード節を拡大解釈しているようです。

「現場で役立つシステム設計の原則」のコード例

まず「現場で役立つシステム設計の原則」(以下、増田本)からアーリーリターンのコード例を紹介します。

この本はとてもわかりやすい、おすすめの本です。

しかし「else句をなくすと良い」は言いすぎであり、誤解する人が多いでしょう。

現場で役立つシステム設計の原則 〜変更を楽で安全にするオブジェクト指向の実践技法 | Gihyo Digital Publishing … 技術評論社の電子書籍
「ソースがごちゃごちゃしていて,どこに何が書いてあるのか理解するまでがたいへん」「1つの修正のために,あっちもこっちも書きなおす必要がある」「ちょっとした変更のはずが,本来はありえない場所にまで影響して,大幅なやり直しになってしまった」とい...

場合分けのコードを整理するもうひとつのやり方は、else 句を書かないことです。else 句は、プログラムの構造を複雑にします。else 句をできるだけ書かないほうがプログラムが単純になります。
具体例で考えてみましょう。「子供」「大人」「シニア」で別料金にするロジックを、else 句を使って書いた例です。

現場で役立つシステム設計の原則 p050

else句を使った書き方(悪い例)

現場で役立つシステム設計の原則 p050
    Yen fee() {
        Yen result;
        if(isChild()){
            result = childFee();
        } else if(isSenior()) {
            result = seniorFee();
        } else  {
            result = adultFee();
        }
        return result;
    }Code language: Java (java)

別に整理しなくても、このままのコードで読みやすいよ?

ローカル変数を使わずに判定後、ただちに結果を返す

現場で役立つシステム設計の原則 p051
    Yen fee() {
        if(isChild()){
            return childFee();
        } else if(isSenior()) {
            return seniorFee();
        } else  {
            return adultFee();
        }
    }Code language: Java (java)

中間変数を使いたくないのなら、ここまではいいと思うけど

else句をなくした書き方(良い例)

現場で役立つシステム設計の原則 p051
    Yen fee() {
        if(isChild()) return childFee();
        if(isSenior()) return seniorFee();
        return adultFee();
    }
Code language: Java (java)

elseをなくすよりも、switch文にしたほうがいいと思うけど

「else句をなくすと良い」は短絡的で、誤解する人が多そうね。ケントベックのSmalltalkベストプラクティスパターン、マーティンファウラーのリファクタリング、ThoughtWorksアンソロジーもやみくもにelse禁止とは言っていないのよ。

増田本の「elseをなくすと良い」は短絡的

マーティンファウラー氏の「リファクタリング」では、elseの重要性を説明しています。

「ガード節による入れ子条件記述の置き換え」のキーポイントを強調しておきましょう。

if-then-else構造が使われるときは、 if部にもelse部にも同じウエイトが置かれています。これは、プログラムの読み手に対して両方とも等しく起こり得ること、等しく重要であることを伝えます。

逆に、ガード節は「めったに起きないが、起きたときには、何がしかのことをやって出て行く」ことを伝えます。

リファクタリング 新装版 p251

elseを使ったほうがいい場合もある、elseを使わないほうがいい場合もあると言っています。

「リファクタリング」では、elseをなくすと良い、とは言っていません。

増田本の「elseをなくすと良い」は、ThoughtWorksアンソロジーのオブジェクト指向エクササイズからの引用です。

その引用元のオブジェクト指向エクササイズでは、ガード節と早期returnを使いすぎないように注意しています。

ただし、メソッドからの早期 return は、使いすぎるとわかりにくくなってしまうことに注意してください。

単純なものであれば、ガード節と早期 return に置き換えられます。

ThoughtWorksアンソロジー 第4章オブジェクト指向エクササイズ p72

オブジェクト指向エクササイズでは、

  • else句禁止にして、if-else構文以外の方法を強制的に考えるようにしよう。
  • 単純なものなら、ガード節と早期return。
  • 複雑なものなら、Strategyパターンとポリモフィズム。
  • NullObjectパターンも試してみて。

と説明しています。

増田本の「elseをなくすと良い(だからアーリーリターン)」は短絡的でしょう。

ThoughtWorksアンソロジーの各章は、ThoghtWorks社のスタッフが担当していて、第2章は、マーティン・ファウラー氏の担当よ。

「リファクタリング」のコード例

次に、マーティン・ファウラー氏の「リファクタリング」(以下、リファクタリング本)から、アーリーリターンのコード例を紹介します。

Amazon.co.jp

死亡したり、離職したり、退職した従業員に対して、特別なルールを持つ給与計算システムを想定します。これらのケースは少ないですが、 ときどき起こります。
次のようなコードがあるとします。

リファクタリング 新装版 第9章 p251

これらのケースは少ないって、

「特別なルールを持つ給与計算システム」の開発をするケースは少ないってこと?

違うわ。死亡したり、離職したり、退職した従業員の給与計算はめったにない、っていう意味ね。

double getPayAmount() {
    double result;
    if (_isDead) result = deadAmount();
    else {
        if (_isSeparated) result = separatedAmount();
        else {
            if (_isRetired) result = retiredAmount();
            else result = normalPayAmount();
        }
    }
}Code language: C++ (cpp)

少しづつ変形しながら、次のコードがファウラー氏の最終形です。

double getPayAmount() {
    if (_isDead) return deadAmount();
    if (_isSeparated) return separatedAmount();
    if (_isRetired) return retiredAmount();
    return normalPayAmount();
}Code language: C++ (cpp)

最初のコードのインデントを調整するだけでいいと思うんだけど

// 最初のコードのインデントだけ調整したバージョン
double getPayAmount() {
    double result;
    if (_isDead) {
        result = deadAmount();
    } else if (_isSeparated) {
        result = separatedAmount();
    } else if (_isRetired) {
        result = retiredAmount();
    } else {
        result = normalPayAmount();
    }
    return result;
}Code language: C++ (cpp)

さて、リファクタリング本のelse句の重要性の説明で、気になる点があります。

「ガード節による入れ子条件記述の置き換え」のキーポイントを強調しておきましょう。

if-then-else構造が使われるときは、 if部にもelse部にも同じウエイトが置かれています。これは、プログラムの読み手に対して両方とも等しく起こり得ること、等しく重要であることを伝えます。

逆に、ガード節は「めったに起きないが、起きたときには、何がしかのことをやって出て行く」ことを伝えます。

リファクタリング 新装版 p251

気になる点をまとめると、

(疑問点)
同じくらい起きるなら、if-then-else
めったに起きないならガード節(アーリーリターン)
を使い分ける

この使い分けに疑問に感じますが、ひとまずおいておきます。

ケントベック「Smalltalkベストプラクティスパターン」のコード例

そして「ガード節」については「ケントベックのSmalltalkベストプラクティスパターン」(以下、ケントベック本)を参照とのことで、見てみると

https://www.amazon.co.jp/dp/4894717549

コミュニケーション・デバイスにまだ接続していない場合、接続するというメソッドがあるとします。一つの出口しか持たないメソッドは、次のようになるでしょう。

connect
self isConnected
ifFalse: [self connectConnection]

このメソッドは、 「もしまだ接続していなかったら、コネクションをはります」と読めるでしょう。同じメソッドをGuardClauseパターンで書くと次のようになります。

connect
self isConnected ifTrue: [^self]
self connectConnection

このメソッドは、 「もし既に接続していたら何もしません。 (通常は)コネクションをはります」と読めるでしょう。GuardClauseパターンを使うと、処理の流れよりも、事実や不変的な事項を浮き彫りにすることができます。

ケントベックのSmalltalkベストプラクティスパターン Gurad Clause p193

架空の言語で書くと、最初のコードは、

    void connect() {
        if (!isConnected()) {
            connectConnection();
        }
    }Code language: JavaScript (javascript)

GuardClause(ガード節)パターンを使ったコードは、

    void connect() {
        if (isConnected()) return;
        connectConnection();
    }Code language: JavaScript (javascript)

GuardClauseパターンのほうが読みやすいわね

ケントベック本の「ガード節」は、とても控えめです。そしてわかりやすい例です。ガード節の使いどころとして、nullチェック以外のいい例だと思います。

リファクタリング本、増田本のコード例は、ガード節ではない

そして、リファクタリング本の

(疑問点)
同じくらい起きるなら、if-then-else
めったに起きないならガード節(アーリーリターン)
を使い分ける

を考えます。

ケントベック本の説明には

もし既に接続していたら何もしません。 (通常は)コネクションをはります。

ケントベックのSmalltalkベストプラクティスパターン

とあります。ガード節で重要なことは「何もしないこと」だと考えます。

リファクタリング本の給与計算を、ケントベック本にならって説明すると、

「もし死亡していたら、死亡者用の給与計算をします。もし転職していたら、転職者用の給与計算をします。もし退職していたら、退職者用の給与計算をします。(通常は)正社員用の給与計算をします。」

ガード節が多いし、処理してからreturnしているわね

それぞれのガード節で何もしていないのではなく、それぞれの給与計算をしています。

ガード節は、めったに起きないことを表現するためではなく、メイン処理が想定していない状態から守るためだと思います。

仮に、ガード節で「めったに起きないこと」を表現したとして、どのようなメリットがあるのでしょうか。

めったに起きないから、死亡者用の給与計算は、テストしなくてもいい?

テストケース書かなくてもいいなら、ガード節で書こうかな♪

ガード節とアーリーリターンが普及した理由って、まさか、それ?

たしかに、めったに起きない、例外を発生させることが面倒、などの理由で、テストケースを書かないと判断することはあります。

しかし、リファクタリング本の給与計算では、死亡者用の給与計算、転職者用の給与計算、退職者用の給与計算のテストケースは書かなくてもいいよね、とはならないでしょう。

つまり、リファクタリング本の給与計算の条件分岐は、ガード節ではない、ということです。アーリーリターンは適切ではありません。

同じ理由で、増田本の子ども、シニア、大人の料金計算の条件分岐も、ガード節ではなく、アーリーリターンを使う場面ではありません。

まとめ

リファクタリング本や増田本の「ガード節」は、ケントベック本の「ガード節」を拡大解釈しすぎているように思います。

仮に、リファクタリング本の最初のコード例で、getPayAmountのメイン処理が「通常の給与計算処理」だったとしたら、(normalPayAmountメソッドを呼ぶのではなく、getPayAmount内でインラインで処理していたら)

double getPayAmount() {
    double result;
    if (_isDead) {
        result = deadAmount();
    } else if (_isSeparated) {
        result = separatedAmount();
    } else if (_isRetired) {
        result = retiredAmount();
    } else {
        〜通常の給与計算処理〜
        〜通常の給与計算処理〜
        〜通常の給与計算処理〜
        result = xxx;
    }
    return result;
}Code language: plaintext (plaintext)

アーリーリターンにしてもいいのでしょうか?

double getPayAmount() {
    if (_isDead) return deadAmount();
    if (_isSeparated) return separatedAmount();
    if (_isRetired) return retiredAmount();

    〜通常の給与計算処理〜
    〜通常の給与計算処理〜
    〜通常の給与計算処理〜
    return result;
}Code language: plaintext (plaintext)

この場合でも、通常ではないケースが3つもありますし、それぞれきちんとメソッドになっていて、処理をしています。それぞのifが成立する頻度は少なくても、ガード節ではないと考えます。

getPayAmount()メソッドから見たら、どのメソッドも同じように重要です。ガード節とアーリーリターンを使う場面ではありません。

ただ、ここで紹介したリファクタリング本のアーリーリターン例と増田本のアーリーリターン例は、それほど悪くないとも思いました。(もとのコードが、リファクタリングが必要なほど、ひどいコードではないですし)

どちらも、switch文とも見てとれるからです。だからこそ、elseを残したままでもいいと思います。else if は、式で分岐するswitch的な立ち位置(Kotlinのwhen的な)だからです。そして、elseをなくすよりも、switchを使うようにリファクタしたほうがいいとも思います。

増田本ではこの後、Mapを使った方法、Enumを使った方法を説明しています。

つまり、ifを使うならアーリーリターン。だけど、なるべくifを卒業して、MapやEnumを使おう、ということを言おうとしているのかしらね。

それにくらべると、リーダブルコードのアーリーリターン例は違和感を感じます。実際のコードのリファクタリングは難しいということもかもれしません。

また、else句のないアーリーリターンはバグの原因にもなりえます。それぞれのifは独立したコードブロックであり、if と if の間に、無関係なコードを挿入できてしまうからです。

アーリーリターンを並べるとインデントが減り、すっきり見えます。すっきり見えることが心地よく、使いすぎといえるほど多くの人が使うようになりました。

でも複雑なものは複雑なんですね。すっきり見えたとしても、本来の複雑さは残ったままです。

それどころか、アーリーリターンは、複数のifを使った手続き的な表現です。if 〜 elseif 〜 よりも改変されやすく、当初の意図とは関係ないコードが追加されやすいことに注意してください。

タイトルとURLをコピーしました