validation - 関数で引数を使用する前に、引数が null でないことを確認する必要がありますか?

Question

タイトルは、私が実際に取得しようとしていることを実際に説明していない可能性があります。

関数が受け入れる引数を使用する前に、null または空であることを確認することをお勧めします。私はこのようにいくつかのハッシュ作成をラップするだけの関数を持っています。

Public Shared Function GenerateHash(ByVal FilePath As IO.FileInfo) As String
        If (FilePath Is Nothing) Then
            Throw New ArgumentNullException("FilePath")
        End If

        Dim _sha As New Security.Cryptography.MD5CryptoServiceProvider
        Dim _Hash = Convert.ToBase64String(_sha.ComputeHash(New IO.FileStream(FilePath.FullName, IO.FileMode.Open, IO.FileAccess.Read)))
        Return _Hash
    End Function

ご覧のとおり、IO.Fileinfo を引数として取り、関数の開始時にそれが何もないことを確認しています。

これは良い習慣なのか、それとも実際のハッシャーに到達させてから、null であるため例外をスローするべきなのか疑問に思っています。

ありがとう。

score 20 · Accepted Answer

一般に、パブリック関数/メソッドのすべての引数を使用する前に検証し、関数の半分を実行した後ではなく早期に失敗することをお勧めします。この場合、例外をスローするのは正しいことです。

メソッドの実行内容によっては、早期に失敗することが重要になる場合があります。メソッドがクラスのインスタンスデータを変更していた場合、オブジェクトのデータが中間の無効な状態にある可能性があるため、データの半分を変更して null に遭遇し、例外をスローすることは望ましくありません。

OO 言語を使用している場合は、パブリックメソッドの引数を検証することが不可欠ですが、プライベートメソッドや保護されたメソッドではそれほど重要ではありません。ここでの私の理論的根拠は、パブリックメソッドへの入力がどうなるかわからないということです。他のコードはクラスのインスタンスを作成し、そのパブリックメソッドを呼び出して、予期しない/無効なデータを渡す可能性があります。ただし、プライベートメソッドはクラス内から呼び出され、クラスは内部的に渡されるすべてのデータを既に検証している必要があります。

score 3 · Accepted Answer

C++ での私のお気に入りの手法の 1 つは、NULL ポインターで DEBUG_ASSERT を行うことでした。これは上級プログラマーによって (const の正確性と共に) 掘り下げられたものであり、コードレビュー中に私が最も厳しくしたものの 1 つです。最初に null ではないと主張せずにポインターを逆参照したことはありません。

デバッグアサートは、デバッグターゲットに対してのみアクティブになるため (リリースでは取り除かれます)、本番環境で何千もの if をテストするための余分なオーバーヘッドはありません。通常、例外をスローするか、ハードウェアブレークポイントをトリガーします。ファイル/行情報とアサートを無視するオプション (セッションで 1 回または無期限) を含むデバッグコンソールをスローするシステムさえありました。これは非常に優れたデバッグおよび QA ツールでした (テスター画面にアサートが表示されたスクリーンショットと、無視された場合にプログラムが続行されたかどうかに関する情報が得られます)。

予期しない null を含め、コード内のすべての不変条件をアサートすることをお勧めします。if のパフォーマンスが問題になる場合は、条件付きでコンパイルし、デバッグターゲットでそれらをアクティブに保つ方法を見つけてください。ソース管理と同様に、これは私を苦しめたというよりも、しばしば私を救ってくれたテクニックです (あらゆる開発テクニックの中で最も重要なリトマス試験紙です)。

score 2 · Accepted Answer

はい、メソッドの先頭ですべての引数を検証し、ArgumentException、ArgumentNullException、ArgumentOutOfRangeException などの適切な例外をスローすることをお勧めします。

メソッドがプライベートで、プログラマーだけが無効な引数を渡すことができる場合、スローの代わりに各引数が有効であることをアサート (Debug.Assert) することを選択できます。

score 1 · Accepted Answer

実用的なプログラマーによるとAndrewHuntとDavidThomasによると、有効な入力を確実に提供するのは発信者の責任です。したがって、null入力を有効と見なすかどうかを選択する必要があります。nullを有効な入力と見なすことが特に意味をなさない限り（たとえば、同等性をテストしている場合はnullを正当な入力と見なすのはおそらく良い考えです）、私はそれを無効と見なします。そうすれば、プログラムが間違った入力にぶつかると、すぐに失敗します。プログラムでエラー状態が発生する場合は、できるだけ早くエラーが発生するようにします。関数が誤ってnullを渡された場合は、それをバグと見なし、それに応じて対応する必要があります（つまり、例外をスローする代わりに、プログラムを強制終了するアサーションを使用することを検討する必要があります。プログラム）。

契約によるクラシックなデザイン：入力が正しければ、出力も正しくなります。入力が間違っている場合は、バグがあります。（入力は正しいが出力が間違っている場合は、バグがあります。それはちょっとしたことです。）

score 1 · Accepted Answer

ブライアンが以前に提供した契約アドバイスによる優れた設計に、いくつかの詳細 (太字) を追加します...

「契約による設計」の原則では、呼び出し元が渡すことを受け入れるもの (入力値の有効なドメイン) を定義し、有効な入力に対してメソッド/プロバイダーが何を行うかを定義する必要があります。

内部メソッドの場合、NULL を有効な入力パラメーターのドメイン外として定義できます。この場合、入力パラメーター値が NOT NULL であるとすぐにアサートします。このコントラクト仕様の重要な洞察は、NULL 値を渡す呼び出しは発信者のバグであり、assert ステートメントによってスローされるエラーは適切な動作であるということです。

さて、非常に明確に定義されて節約されていますが、メソッドを外部/パブリック呼び出し元に公開している場合は、それが私/私たちが本当に望んでいる契約ですか? おそらくそうではありません。パブリックインターフェイスでは、おそらく NULL を受け入れますが (技術的にはメソッドが受け入れる入力のドメインで)、戻りメッセージを使用して適切に処理することを拒否します。(当然より複雑な顧客対応の要件を満たすために、より多くの作業が必要になります。)

どちらの場合でも、呼び出し元とプロバイダーの両方の観点からすべてのケースを処理するプロトコルが求められます。契約の完全性または完全性の欠如を評価することを困難にする可能性のある多くの散発的なテストではありません。コンディションカバー。

score 1 · Accepted Answer

NULL が受け入れられない入力である場合は、例外をスローします。メッセージが役立つように、サンプルで行ったように、自分で。

NULL 入力を処理する別の方法は、順番に NULL で応答することです。関数のタイプによって異なります。上記の例では、例外を保持します。

score 1 · Accepted Answer

外部向けの API の場合は、入力が信頼できないため、すべてのパラメーターを確認する必要があります。

ただし、内部でのみ使用する場合は、入力を信頼できる必要があり、ソフトウェアに価値を追加しないコードの束を節約できます。

score 1 · Accepted Answer

すべての引数を、それらの値についてその関数で行う一連の仮定に対してチェックする必要があります。

あなたの例のように、関数への null 引数が意味をなさず、関数を使用する人なら誰でもこれを知っていると想定している場合、null 引数が渡されると、何らかのエラーと何らかのアクションが実行されます (例: . 例外をスローします)。また、アサートを使用する場合 (James Fassett が参加して私の前に言ったように ;-))、リリースバージョンではコストはかかりません。(デバッグバージョンでもほとんど費用はかかりません)

同じことが他の仮定にも当てはまります。

また、エラーを生成した方が、標準ライブラリルーチンに任せて例外をスローするよりも、エラーを追跡するのが簡単になります。より有用なコンテキスト情報を提供できるようになります。

この質問の範囲外ですが、関数が行う仮定を公開する必要があります-たとえば、関数へのコメントヘッダーを介して。

score 0 · Accepted Answer

ほとんどの場合、例外が無視されないことが確実である限り、例外をスローするだけで十分です。

ただし、何かを追加できる場合は、例外をより正確なものでラップして再スローしても問題はありません。「NullPointerException」のデコードには、「IllegalArgumentException("FilePath MUST be supply")」(または何でも) よりも少し時間がかかります。

最近、テストする前に難読化ツールを実行する必要があるプラットフォームに取り組んでいます。すべてのスタックトレースは、サルがランダムにがらくたを入力しているように見えるので、引数を常にチェックする習慣を身につけました。

コンパイラがチェックできるように、変数と引数に「nullable」または「nonull」修飾子を付けたいと思います。

score 0 · Accepted Answer

パブリック API を作成している場合は、呼び出し元がバグをすばやく見つけられるように支援し、有効な入力を確認してください。

呼び出し元 (または呼び出し元の呼び出し元) が信頼できない可能性がある API を作成している場合は、有効な入力を確認してください。これはセキュリティが優れているためです。

C# の「内部」のように、信頼できる呼び出し元だけが API にアクセスできる場合は、余分なコードをすべて記述する必要はありません。誰の役にも立ちません。

validation - 関数で引数を使用する前に、引数が null でないことを確認する必要がありますか?

10 に答える 10

Related

Reference