関数の引数はどこで検査するべきか

Question

/**
* 自然数同士の足し算を行う
*
* @param integer $x 自然数であること
* @param integer $y 自然数であること
* @return integer $xと$yの合計値
*/
function Add($x, $y) {
    $sum = $x + $y;
    return $sum;
}

このようなコードがあるとします。
$x, $yが引数として妥当であること（型がintegerである、自然数である）の検査をする箇所について質問があります。
検査すべき場所として下記3パターンあると思うのですがどれが妥当なのですか？
(a)関数内で行うもの
(b)呼び出し側で行うもの
(c)a,b両方で行うもの

防衛的プログラミングや契約プログラミングというワードを「達人プログラマー」の書籍でしり混乱しています...
上記3パターン以外の手法も歓迎しております。どうかお知恵をお貸しください。

【追記】
情報の質と量が多くみなさんの全ての回答を読めていません。
いまだ「未解決」のままになっていますが週末利用して頂いた回答を理解できるように致します。

Answer 1

和田卓人さんの講演が参考になると思います。長い（45分）ですが、それだけの価値があるものです。私は本番も聴講していますが、加えて何度もビデオで見返しました。

PHP7で堅牢なコードを書く - 例外処理、表明プログラミング、契約による設計 和田 卓人（ビデオ）
PHP7で堅牢なコードを書く - 例外処理、表明プログラミング、契約による設計 和田 卓人（スライド）

Comments

[m.ts10806]

（横からごめんなさい）
こんな素晴らしいものが動画になってるんですね。
ちょうどしっかり押さえたい箇所だったので参考にさせてもらいます。

[退会済みユーザー]

この資料、最初に見た時に衝撃を受けましたｗ

[yambejp]

いい資料をありがとうございます。
例外を投げればいいってもんじゃないという考え方は頷けるものでした
とくにアサーションは利用していなかったので、PHP7以降の有効な機能として注目ですね

[ms90]

資料紹介ありがとうございます。
長くてもためになるものでしたら休みにじっくりみてみます。
例外について理解したいので大変助かります。

Answer 2

これはとっても難しい問題で、賛否両論ある議題でもあるんですよね。
そんな中で、持論を書いてみます。

まず、その関数がどの業務にも依存しない共通のユーティリティー的な立ち位置なのであれば、
防衛的プログラミングをした方が、呼び出し側もシンプルになり使いやすいものになると思います。

(a)関数内で行うもの

こちらに該当します。

ただ、型の問題（自然数である前にまずは整数であること）は、
maisumakunさんがおっしゃっているタイプヒント一択ではないでしょうか。

次に、アプリケーション設計を各レイヤーに分けている場合です。
レイヤーの定義は以下のものを前提にします。（ググるとたまたま一番上にきただけの理由です）
アプリケーションのレイヤ化

アプリケーション層からドメイン層のサービスを呼ぶ場合の引数などは
関数内ですべきではありません。

(b)呼び出し側で行うもの

こちらに該当します。

なぜなら、ドメイン層への入力は決められた業務の情報であり、
それ以外での呼ばれ方を意識すべきではないからです。（あり得ない業務を意識するのはおかしい）
つまり、想定外の呼ばれ方をした場合は、エラーが起きるべきだということです。
本当に良く見るのはドメイン層でのnullチェックですね。
そもそも業務上nullであること自体がおかしいのだからエラーにならなければおかしいでしょう。
そしてこのエラーはドメイン層のバグではないということが重要です。
nullエラーになったからといって、関数内でnullチェックを入れるのはおかしいということです。

入力情報の検証などは、アプリケーション層の役割です。
そこで業務に合った検証を行い、不適切であればその時点でエラーにする。

「入力情報の検証」といっても、数字であること、日付であること、などの単純なものと
業務的な検証（例えば在庫のチェック）とかで、また話しは変わってきますが、
「自然数であること」などはアプリケーション層の役割でしょう。

(c)a,b両方で行うもの

こちらに関しては、全く必要性を感じません。
関数内で行っているのに、呼び出し側でも行っているのは、
そのプログラマーが、ただ単にその関数の仕様を分かっていないだけのことでしょう。
ただ関数を作成する側も、仕様が分かるように丁寧にコメントを残すなどの配慮は必要だと思います。

コメントの必要性や、粒度も色々と議論があるようですが、
最低限、クラス、インターフェース、関数などの仕様の説明は絶対に必要です。
みなさんそれらを見て、OSSなどを利用しているわけですから。

Comments

[ms90]

解答ありがとうございます。
＞nullエラーになったからといって、関数内でnullチェックを入れるのはおかしいということです。
こちらの例は関数内では引数の正当性について一切判定を行わないということでしょうか？
「万が一（そもそも例のルールでは実装ミスになるかと思いますが）引数に不正な値が紛れれば関数は予想外の行動をする。けれどこれは呼び出し側を正しく実装していれば防げる。」と受け取りました。

[root_jp]

> 引数に不正な値が紛れれば関数は予想外の行動をする。
ありえない業務のための制御を入れることは、アプリケーション全体の設計バグを隠蔽することにつながるため、早々にエラーにしてプログラムを停止した方が良い時もあると言っています。
呼び出し側を正しく実装というよりは、設計が間違っていなければおおよそは問題にならない。
と言った方がいいかもしれません。

例えば、「会員ステータス」のような情報があったとして、「有効」「退会」「停止中」などの状態があるとします。
if (!is_null(会員ステータス)) とかいう分岐おかしいでしょう。
ユーザーが会員になった時点で、何らかのステータスにはなるわけですから、
ステータスがnullなんか設計上ありえないんですよ。
ありえてるってことは、もっと根本的にどこかがバグってるってことですから、
上記のような if はその根本原因を隠蔽しているだけの害でしかないこともあると言っています。

[root_jp]

肝心の質門に答えてない気がしたので、連投失礼します。
>こちらの例は関数内では引数の正当性について一切判定を行わないということでしょうか？
こちらの例ではそういうことです。プログラムが落ちるのが正しいです。
入力情報の検証などはアプリケーション層（呼び出し側）できちんと行ってから、ドメイン層には流してくださいねってことです。
なので「呼び出し側を正しく実装していれば防げる」とう認識は一部合っています。

Answer 3

＞契約プログラミング　→　(b)呼び出し側で行う
＞防衛的プログラミング　→　(a)関数内で行う

ご質問の部分に対して、上記のような対応になります。
この防衛と契約で何が違ってくるのか、意味を少し考えてみます。

エラーチェックは何も考えずにやると、ものすごく負担になります。
想定外の想定はキリがないので、際限なく複雑になりうる。

単純に、関数が100個あったら、100回チェックすれば、何倍にもなります。
＞(c)a,b両方で行う　などと重複してたら、もっと増えてしまうでしょう。

そこで、事前条件の確保は呼び出し側の責務と考え、
違反していたときに例外で突き返して良いとすることで、
複雑になりがちなエラー処理をシンプルにできます。

防衛より契約の方がモダンな手法だと思います。
治安が良いと自衛の負担が少ない、とかそういうイメージです。

---

もう少し踏み込んだ内容も見てみます。「契約プログラミング」は難解なので、
以下は中途半端な解説になりますが、参考までに流し読みしてください。

メイヤーの契約概念は例外処理と（だけ）結びつけて語られやすいですが、
メイヤー流の見方では、そもそもオブジェクト指向そのものが、
抽象データ型を利用した契約の体系（公理的・代数的な体系）なのです。

紙幅が足りないので、極度に単純化したものとお断りしておきますが、
その考え方の一部だけ言うと、たとえば不変条件をクラスの単位だと考えます。

どういうことかというと、処理を書いてから検証を考えるというより、
処理の前提となる基準（不変条件）が共通するように、最初からクラスを作っておきます。

加算関数や減算関数などが各自バラバラに自然数の条件を検証するのではなく、
自然数クラスに加算メソッドや減算メソッドを付けていく感じです。

OOPだとコンストラクタやゲッター／セッター、例外／表明などの仕組みがあり、
データを検証しやすいのです。コンストラクタやセッターで
値を監視しておけば、負の値にならないことなどは保証できます。
関数ごとにチェックするコードを全部書いて回る必要がない。

たとえれば、取引所に関係者だけ集めて取引すると、
スムーズに話が進むみたいなイメージでしょうか。

そういう感じで、OOで上手く分析・設計できれば（これが難しいけど）、
重複を省くことでチェックするコードを劇的に減らせます。
ただ、高度なOOは入門書や入門サイトに全く書いてなく、非常に難しいです。

処理よりデータを中心に考えることに、違和感を覚えるかもしれませんが、
「DDD（ドメイン駆動開発）」なども、こうした発想の延長にあります。

Comments

[ms90]

解答ありがとうございます。気になることがあったのでもう少し教えてください。
>そこで、事前条件の確保は呼び出し側の責務と考え、
>違反していたときに例外で突き返して良いとすることで、
>複雑になりがちなエラー処理をシンプルにできます。
例外を投げるということは関数内でもチェック処理は書くということですか？これは(c)のパターンには当たらないという認識なのでしょうか？

[LLman]

自然数のような簡単な例だと違いが分からないと思いますが、
複雑な処理だと例外チェックの方が簡明になります。

実務レベルのアプリでは、チェックするだけでなく、
チェックで間違っていたら修正する必要があります。
エラーのたびに落とすわけにいかない場合が多々ありますから。

具体的に言うとたとえば、null（nil）だったときに、
値を初期化するといった修正・回復する仕組みです。
あるいは入力が失敗したときに何回か繰り返すとか。

それで、そういう修正は、目的の関数内ではやらない、
呼び出し元で済んでいる、という前提を置けるだけでも、だいぶスッキリします。

[ms90]

呼び出し側で引数の妥当性をチェック、必要あれば修正、回復処理をする。
関数内では引数の妥当性をチェック、期待しないものであれば例外を投げる。

というのがLLmanさんが私にアドバイスしてくださっていることでしょうか
これが絶対正ということを決めつけたいのではなく、頂いているアドバイスの意図を理解したいために確認しています。
しつこくて申し訳ないです。

[LLman]

＞呼び出し側で引数の妥当性をチェック、必要あれば修正、回復処理をする。
＞関数内では引数の妥当性をチェック、期待しないものであれば例外を投げる。
そうです。呼び出し側に事前条件を確保する責務があるからです。

しかし、ms90さんに、
「なんだ結局、引数チェックを両方に書いてるじゃん」
という違和感が残るかもしれません。

もう少しフォローすると、本文で書いたOOの仕組みを使う手もあるし、
もっと簡単な方法では妥当性をチェックする別の関数を呼ぶようにすれば、
一行で済むので、そこの重複は大した手間ではありません。

では、どっちが例外でどっちが回復かが、なぜ大事かというと、
複数人の開発、とくにAPIやライブラリのようなものだと、
呼び出し側と呼ばれる側で開発組織が異なる場合があるからです。

その場合、外側からどういう処理をされるのか全く不明になり、
修正や回復が難しい場合がよく生じます。そういう時には、
作るものの全体像が見えている呼び出し側の方が回復しやすい。

そもそも「契約」とは、複数の人間間で交わされるものなので、
複数人の開発にそういう概念を持ち込むことは有効なわけです。

だからじつは、ひとりで組む前提なら、どっちかのチェックを端折っても、
大した問題でないことも多いです。とくに小規模なら、ひとりで全体を把握できるので。

ただたとえば、一年後の自分は他人同然で、構造を理解できない可能性もあります。
やりたいことが変わって、関数の配置が離れてしまうこともよくあるでしょう。
だから、たとえひとりの開発でも、他人行儀なルールの採用に意味はあると思います。

Answer 4

それ以前に、条件にあわないときにどういった処理をしたいかによるのでは？

Addがエラーを返したり、想定外のデータを受けたときに別に置き換えてやるなら
Add内で完結してよいでしょう。
もちろん事前にデータをチェックしたとしても、Add側で二重にチェックしてやるほうが
より確実だと思います

追記

考え方

• パターン1

function Add($x, $y) {
    $sum = $x + $y;
    return $sum;
}
function is_natural(){
  $args=func_get_args();
  $ret=true;
  foreach($args as $num){
    if(!is_int($num) or  $num<=0) $ret=false;
  }
  return $ret;
}
$x=10;$y=20;
if(is_natural($x,$y)){
  print Add($x,$y);
}else{
  print "wrong data!";
};
$x=-10;$y=20;
if(is_natural($x,$y)){
  print Add($x,$y);
}else{
  print "wrong data";
};

※上記のように、外側でチェックをすればAddから戻るデータはかならず自然数です。
型が保証されるのでわかりやすいでしょう。
逆に例外処理を都度書かなくては行けないのは煩雑です。

• パターン2

$x=10;$y=20;
print Add($x,$y);
$x=-10;$y=20;
print Add($x,$y);
function Add($x, $y) {
  if(!is_natural($x,$y)) return "wrong data!";
  $sum = $x + $y;
  return $sum;
}
function is_natural(){
  $args=func_get_args();
  $ret=true;
  foreach($args as $num){
    if(!is_int($num) or  $num<=0) $ret=false;
  }
  return $ret;
}

メインパートは書きやすくなりますが、Addからの戻り値の型が曖昧になります

errorでthrowしたりすることも視野にいれれば
個人的にはパターン1の方がデータ管理がし易いとおもいます。

Comments

[yambejp]

一応パターンわけを追記しておきました

[ms90]

呼び出し側で都度チェックし、偽なら例外をなげてハンドラで一括処理みたいなイメージですかね。
サンプルで説明していただきありがとうございます。

Answer 5

似たような質問がTeratail上にあるので、一度目を通しておくと良いでしょう。
実行制限のある関数の記述方法

PHP7ではプリミティブ型のタイプヒンティングが可能ですが、
これは関数のあり方を宣言しているだけなので、関数側での対策には入らないと考えています。
当然あった方が良いと思います。

基本は「プロジェクトの決定に従う」が正解です。
個々が協力せず、自分の思想を勝手にコードに反映すると反発しあった汚いコードになります。
当事者同士で納得行くまで話し合って決定しましょう。

ですので、その決定を作る為に議論している状況と仮定します。
その場合b > a >> cで考えています。
下記、例を交えつつ解説します。

---

あなたが部下や同僚に仕事を依頼したとしましょう。
仕事を完遂させるには、前提条件となる資料（引数）が必要です。
もし資料不足で部下の仕事が失敗したら誰の責任になりますか？

aの設計は仕事を依頼された側に責任があるから、よしなに解決しろと言っています。
bの設計のみ仕事の依頼元の責任と言ってます。
私が依頼された側だったらイラッとするのでbにしてほしいです！

モダンな言語の多くでは、依頼先（関数の中身）は例外投げて死ねばいいという設計です。
これは仕事の依頼者（呼び出し元）の責任でハンドリングしろと言っています。
また、多くの言語のビルトイン関数も引数が不正な場合、エラーや例外を投げて死ぬ挙動になっています。

自分たちの作った関数だけ急に呼び出し先が頑張る思想にするとあべこべになりますから、
呼び出し元がハンドリングを頑張るのが自然な設計かと思います。

---

aは典型的な防衛的プログラミングですね。
これはルールを厳格に行い、用法・用量を守って適切に扱いましょう。
そうでなければプロジェクトが悲惨な目にあいます。

関数を呼ぶ時に「Stringが来たらどうすんだ？」や「合計するとInt型の上限値を超える値を渡したらどうなるんだ？」
…という重箱の隅をつつくようなケースもやり玉に挙げられる可能性があります。

それらの対応を全て行うときりがありません。
簡単な2つの値を足すだけの単純な処理が、例外処理だらけで20行くらいになってる関数を使いたいですか？
それが全ての関数に適用されている状況を想像してみてください。
もし私がそれをメンテし続けろと命じられたら、その日の内に退職願いを叩きつけるかもしれません。

なので匙加減のハンドリングをし続ける努力が必要になります。
私は面倒なので、あまりやりたくないのでb推奨派です。

一番危ないのはユーザーからの入力値ですので、
そこさえ死ぬ気で見張れば、他の処理はあまり神経質になる必要はないと思ってます。

Comments

[ms90]

質問の紹介ありがとうございます。

>また、多くの言語のビルトイン関数も引数が不正な場合、エラーや例外を投げて死ぬ挙動になっています。
Warning出してfalse返してくる関数等のことですね。
少し謎が解けた気がします。

[root_jp]

警告は続行可能ですが、エラーは続行不能のことです。
return false と throw new InvalidArgumentException() は全く別物ですよ。

[miyabi-sun]

> Warning出してfalse返してくる関数等のことですね。
認識は合ってますが、PHPの失敗するとfalseを返す関数群はあまり良い実装ではないですね。
後付で成功したか失敗したかをif文で取得しにいくなら最初から通るケースだけちゃんと振り分けとけって話ですね。

かと思えばPDOみたいにすぐに例外吐いて死ぬモダンな設計になっている箇所もありますね。
一貫してません。
PHPは下位互換を重視する文化なので昔から存在する関数の挙動や名称を変えられないのが一番の弱みですね。

Answer 6

人為的に検査しなくても、PHP 7であれば、スカラー型についても引数のタイプヒントを宣言できます（リファレンス）。自分で実装しなくて済むことは、処理系に任せてしまいましょう。

// int以外が来たらTypeError例外
function Add(int $x, int $y) {
    $sum = $x + $y;
    return $sum
}

Comments

[maisumakun]

なお、PHP特有の「ゆるさ」は健在ですので、こう書いても（別途でdeclareしない限り）「整数として解釈できる文字列」や「小数点以下がゼロの浮動小数点数」などは型変換されて受け付けられるような挙動となります。

[ms90]

回答ありがとうございます。
仰るとおり引数にint指定してやることで整数であることは担保できると思います。
ですが依然として自然数であることは担保されておらず、呼び出し側、関数内のどちらかに条件文を書いて検証する必要があると思うのです。
それをどこでするべきがご意見を聞かせていただきたいというのが私の質問の意図です。分かりにくく申し訳ありません。