ケントベック「Smalltalkベストプラクティスパターン」のガード節とアーリーリターンは、とてもシンプルで読みやすくなるコード例です。それに比べると「リファクタリング」や「現場で役立つシステム設計の原則」のアーリーリターンのコード例は、ケントベックのガード節を拡大解釈しているようです。
「現場で役立つシステム設計の原則」のコード例
まず「現場で役立つシステム設計の原則」(以下、増田本)からアーリーリターンのコード例を紹介します。
場合分けのコードを整理するもうひとつのやり方は、else 句を書かないことです。else 句は、プログラムの構造を複雑にします。else 句をできるだけ書かないほうがプログラムが単純になります。
現場で役立つシステム設計の原則 p050
具体例で考えてみましょう。「子供」「大人」「シニア」で別料金にするロジックを、else 句を使って書いた例です。
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 は、使いすぎるとわかりにくくなってしまうことに注意してください。
ThoughtWorksアンソロジー 第4章オブジェクト指向エクササイズ p72
単純なものであれば、ガード節と早期 return に置き換えられます。
オブジェクト指向エクササイズでは、
- else句禁止にして、if-else構文以外の方法を強制的に考えるようにしよう。
- 単純なものなら、ガード節と早期return。
- 複雑なものなら、Strategyパターンとポリモフィズム。
- NullObjectパターンも試してみて。
と説明しています。
増田本の「elseをなくすと良い(だからアーリーリターン)」は短絡的でしょう。
ThoughtWorksアンソロジーの各章は、ThoghtWorks社のスタッフが担当していて、第2章は、マーティン・ファウラー氏の担当よ。
「リファクタリング」のコード例
次に、マーティン・ファウラー氏の「リファクタリング」(以下、リファクタリング本)から、アーリーリターンのコード例を紹介します。
死亡したり、離職したり、退職した従業員に対して、特別なルールを持つ給与計算システムを想定します。これらのケースは少ないですが、 ときどき起こります。
リファクタリング 新装版 第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
気になる点をまとめると、
この使い分けに疑問に感じますが、ひとまずおいておきます。
ケントベック「Smalltalkベストプラクティスパターン」のコード例
そして「ガード節」については「ケントベックのSmalltalkベストプラクティスパターン」(以下、ケントベック本)を参照とのことで、見てみると
コミュニケーション・デバイスにまだ接続していない場合、接続するというメソッドがあるとします。一つの出口しか持たないメソッドは、次のようになるでしょう。
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チェック以外のいい例だと思います。
リファクタリング本、増田本のコード例は、ガード節ではない
そして、リファクタリング本の
を考えます。
ケントベック本の説明には
もし既に接続していたら何もしません。 (通常は)コネクションをはります。
ケントベックの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を使うようにリファクタしたほうがいいとも思います。
つまり、ifを使うならアーリーリターン。だけど、なるべくifを卒業して、MapやEnumを使おう、ということを言おうとしているのかしらね。
それにくらべると、リーダブルコードのアーリーリターン例は違和感を感じます。実際のコードのリファクタリングは難しいということもかもれしません。
また、else句のないアーリーリターンはバグの原因にもなりえます。それぞれのifは独立したコードブロックであり、if と if の間に、無関係なコードを挿入できてしまうからです。
アーリーリターンを並べるとインデントが減り、すっきり見えます。すっきり見えることが心地よく、使いすぎといえるほど多くの人が使うようになりました。
でも複雑なものは複雑なんですね。すっきり見えたとしても、本来の複雑さは残ったままです。
それどころか、アーリーリターンは、複数のifを使った手続き的な表現です。if 〜 elseif 〜 よりも改変されやすく、当初の意図とは関係ないコードが追加されやすいことに注意してください。