Q&A
言語によって、あとそのメソッドを使う人の想定によっては、だいぶ回答内容がぶれると思うので、その2点は明記した方が良いと思います。
お世話になります。
メソッド、関数の引数チェックを呼び出し先で行うか、呼び出し元で行うかの設計で悩んでいます。
例えば、nullチェックのような処理がエラーになるような場合のチェックはメソッド内で行い、例外をthrowしたりすると思います。
では、状態チェックメソッドで受け取った５文字の文字列とチェックするような場合、文字列が５文字であるかどうかをメソッド側で５文字でないと例外を返す方がいいのか、単純に不一致として返すか、またはチェック処理前に文字列を５文字分切り出す処理を入れる方が便利でしょうか。
基本的にメソッド側でチェックするとして、どの程度厳密にチェックするのかというのも明確な判断ができていません。
当然、ケースバイケースということになると思いますが、
そのあたりの設計の指標を教えていただけるとありがたいです。
自分で調べた感じでは以下
・メソッドの場合、それがpublicかprivateか
publicの場合、厳密にチェック、privateの場合はエラーチェックはassertを使う？
・受け取った引数が、ユーザーからの入力値のような外部要因のものかどうか
外部の場合はバリデーション含めて、チェック
よろしくお願いします。
Validationはシステムの設計思想に深く関わります。設計者が自分である場合だと仮定しても使用するフレームワークなどで変わってくるために一概に回答できないです。
回答2件
2
ベストアンサー
こんにちは。
メソッド、関数の引数チェックを呼び出し先で行うか、呼び出し元で行うかの設計で悩んでいます。
一般に関数は複数の箇所から呼ばれるでしょうから、通常は関数側でチェックした方が良いように感じます。１箇所からしか呼ばれない関数ならどちらでチェックしてもよいように思います。パラメータを増やさずにチェックとエラー報告ができるなら、関数側でチェックしておくと他の場所からも呼びたくなった時に楽できるかも知れません。
当然、ケースバイケースということになると思いますが、
そのあたりの設計の指標を教えていただけるとありがたいです。
指標として妥当かどうか分かりませんが、私は概ね以下のようなイメージで対処してます。
例外
続行不可能なエラーや本来有り得ないエラーはできるだけ例外を投げてます。バグかリソース不足が多いです。（このエラー対処用のコードの数が一番多くなると思います。）
そして、できるだけ大本でcatchしてログに残すようにしてます。C#はスタック・トレースがお手軽なのでありがたいです。C++も最近スタック・トレースが標準化されたらしいので使う機会を待っているところです。
アサート
例外を投げることができない場合はアサートすることがあります。例えばC++でデストラクタで検出した続行不能エラーで実稼働中に発生することが考えにくいケースなど。
エラーコード
操作ミスなら、オペレータに通知することになりますね。その時は、エラーコードで返却した方がオペレータに通知して対処を促しやすい場合が多いように感じます。（常に成り立つわけではないですが。）
ただし、ハードウェアの故障やハードウェアの接続ミス等も基本はオペレータに通知して対処して貰うことになりますが発生頻度は低いですし、続行不能な場合が多いですから、例外を使うことが多いですね。
投稿2018/02/26 14:00
総合スコア23274
コメントありがとうございます。
「できるだけ大本でcatch」というのは、例えばwebアプリケーションでユーザー情報を登録する処理で本来あり得ないエラー(プログラムのバグ)があった場合、それをcatchするのはモデルクラス、コントローラークラスどちらになるのでしょうか。
後、プログラムのバグなどによる続行不可能なエラーは単体テストで保証するもので、本番環境では例外処理はすべて外すものでしょうか。
「できるだけ大本でcatch」はプログラミング言語とフレームワークの仕様上可能な大本です。
これは本質的に必要悪ですからできるだけコード量が少ない方が好ましいですし、catch漏れの可能性も減らしたいです。
> プログラムのバグなどによる続行不可能なエラーは単体テストで保証するもので、本番環境では例外処理はすべて外すものでしょうか。
もしも、バグを撲滅できるなら本番環境で外すこともありえますが、現実の世界でバグの撲滅は夢物語です。従って、本番環境で外すのはナンセンスと考えています。寧ろ、デバッガが無い上、再現できるとは限らないので本番環境でこそ威力を発揮します。
ありがとうございます。
少し話がそれますが、本番環境でバグが発生した場合にロギングすると思うのですが、
ロギングの処理ってどこで行っているものですか。これも「できるだけ大本でcatch」のところでまとめて行う感じでしょうか。
その通りです。これはリカバリが目的ではなく本番環境も含めて顕在化したバグのデバッグに有用な情報の収集が目的です。
ありがとうございます。参考になりました。
1
似たような質問がありましたので、一応参考までにリンクを貼り付けておきます。
実行制限のある関数の記述方法 - Teratail
そもそも不正と分かっている値を使って、
関数やメソッドの引数に投げ込むのはお行儀が良いプログラムだとは思いません。
DBには常に既に綺麗な値が入っているべきなので基本的には信頼しても良い、見張るのはユーザーからの入力値だけで良いと思います。
それでも失敗しそうなリスクを抱えているなら(例えばSQLがエラーになる、DBやHTTP通信が失敗する等)
tryで受け取れるように準備しておくべきかなと考えています。
この前提を元に回答する形になります。
状態チェックメソッド
素直に一致しなければ全部falseで良いと思います。
次点は型が違う、null、5文字以外のケースなら即例外ですかね。
メソッドや関数はシンプル低機能なのが一番です。
高機能であればあるほど使いにくい負の遺産になると考えてください。
何故ならば、高機能になればなるほど外からは何やってんのか見えなくなります。
つまり外から中が透けて見える関数やメソッド名にする必要があります。
余り端折りすぎた名前を付けるのは、未来の自分や同僚に「ファルシのルシがパージでコクーン」を押し付ける事になります。
先頭の5文字を直訳すれば「The beginning of the 5 characters」で、既に長すぎでしょう。
これに更にチェックを行いtrue or falseを返すシンプルな名前を考えると辛いですね。
関数型プログラミングではtake 5がString型なら先頭から5文字を抜き出すという意味で使われる事が多いです。
定義されたコード一覧に含まれるという感じでin declared codeでしょうか？
うーん、私の英語力じゃこんなもんですか、色々考えてみてください。
JavaScript
1var take = function (num, str) { 2 if (str.length === num) return str; 3 if (str.length > num) return str.slice(0, num); 4 return (Array(num).fill('0').join('') + str).slice(0 - num); 5} 6var inDeclaredCode = function (str) { 7 var codes ['00000', '00001']; 8 return (codes.indexOf(str) !== -1); 9} 10var str = "12345"; 11var result = inDeclaredCode(take(5, str));
投稿2018/02/26 12:38
総合スコア21418
コメントありがとうございます。
型、null、５文字以外を例外にするというのは開発中にテスト、デバッグする用にということでしょうか。
本番系ではこういった例外処理は外しているものでしょうか。
本番環境でも処理は変えてはいけません。
もしthrowを投げて殺す設計に決めたのなら、開発環境も本番環境も同様の動作にしてください。
動作を変えた場合のデメリットを幾つか挙げていきます
- コードがif文まみれになり可読性が落ちる
- 変更された箇所は実際に動作するか保証できない
- 5文字のStringくれって言ってるモジュールに、本番環境にもなって平気でInt型やNullを突っ込んで動かそうとしてくるシステムって相当ヤバイ
テストコード等をしっかり書いて管理していくのが今の理想ですね。
すいません、「動作を変えた場合」というのは本番環境で例外処理を外した場合という意味でしょうか。
それともメソッドの動作を変更した場合ということでしょうか。
挙げていただいている３つのデメリットが何の話に掛かっているのかがちょっと分からなかったです。
想定していたのは、
環境変数等の外部情報を元に値がproduction等の本番環境用の値であった場合は
if文等で分岐させてチェック処理を省略したりthrowを投げないという方法です。
開発環境用と本番環境用のコードは確実に同じものを使ってください。
限許容出来るのはconfig.txtなりやconfig.php等の変数や定数を宣言出来るものだけです。
全てのモジュールでやると確実にどっかバグ埋め込みます。
なるほど、そういうことですね。
ああ、今までデバッグモードみたいな感じで(configファイルの定数で切り替え)、開発と本番系でif文で処理を分岐させるような設計を使ったことがありますが、あれって駄目なんですね。あまり問題にならなかったのですが、小規模だったからかもしれないです。チェック処理の省略とかではなくて、外部のサーバーとかapiとかのやりとりを省略したり、仮の振る舞いをするようにしていたのですが。
本来は共通のインタフェースを持つ、フェイク用のクラスを渡してあげる方がいいみたいですね。
既存の案件とかで、兎に角自分のモジュールの中身だけ守らなければならないとか
理想論だけ語ってもしょうがないケースはあると思います。
出来るだけ勝手に動きが変わるものは少なくした方が楽ですって感じですかね。
> 外部のサーバーとかapiとかのやりとりを省略したり、仮の振る舞いをするようにしていたのですが。
コードを変えずに頑張るという場合、本当に通信しちゃうってのも一つの手だと思います。
仮の値を返す鯖をNode.jsかなんかででっち上げて、ローカル開発だとそっち使うという手段もありはします。
いまだとdocker-composeでこういったNode.jsの鯖を包んで動かせますからね。
大変参考になりました。ありがとうございます。
あなたの回答
tips
プレビュー
