どの程度防御的にプログラムすればよいでしょうか?[閉まっている]

https://stackoverflow.com/questions/1053215

20-08-2019
|

質問

私はデータベース接続を作成するために使用される小さなルーチンを使用していました。

前に

public DbConnection GetConnection(String connectionName)
{
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
   DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);
   DbConnection conn = factory.CreateConnection();
   conn.ConnectionString = cs.ConnectionString;
   conn.Open();

   return conn;
}

それから私は、.NET Framework のドキュメントを調べ始めて、何が含まれているかを確認しました。 文書化された さまざまな動作があり、それを処理できるかどうかを確認します。

例えば：

ConfigurationManager.ConnectionStrings...

のドキュメンテーション電話をかけていると言う 接続文字列 を投げます構成エラー例外コレクションを取得できなかった場合。この場合、この例外を処理するためにできることは何もないので、そのまま放置します。

次の部分は、実際のインデックス作成です。 接続文字列 見つけるには 接続名:

...ConnectionStrings[connectionName];

この場合、 ConnectionString のドキュメント財産は戻ってくると言うヌル接続名が見つからなかった場合。これが起こっているかどうかを確認し、例外をスローして、無効な connectionName を指定したことを誰かに知らせることができます。

ConnectionStringSettings cs= 
      ConfigurationManager.ConnectionStrings[connectionName];
if (cs == null)
   throw new ArgumentException("Could not find connection string \""+connectionName+"\"");

同じ練習を次のように繰り返します。

DbProviderFactory factory = 
      DbProviderFactories.GetFactory(cs.ProviderName);

の工場取得メソッドには、指定されたファクトリが存在した場合に何が起こるかについてのドキュメントがありません。 ProviderName 見つかりませんでした。返品することが文書化されていない null, 、でも私はまだ防御的になることができます、そして チェック nullの場合:

DbProviderFactory factory = 
      DbProviderFactories.GetFactory(cs.ProviderName);
if (factory == null) 
   throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");

次に、DbConnection オブジェクトの構築です。

DbConnection conn = factory.CreateConnection()

もう一度、ドキュメンテーション接続を作成できなかった場合に何が起こるかについては述べていませんが、ここでも null 戻りオブジェクトを確認できます。

DbConnection conn = factory.CreateConnection()
if (conn == null) 
   throw new Exception.Create("Connection factory did not return a connection object");

次に、Connection オブジェクトのプロパティを設定します。

conn.ConnectionString = cs.ConnectionString;

ドキュメントには、接続文字列を設定できなかった場合に何が起こるかについては記載されていません。例外がスローされますか?それは無視しますか？ほとんどの例外と同様、接続の ConnectionString を設定しようとしたときにエラーが発生した場合、それから回復するためにできることは何もありません。だから私は何もしません。

最後に、データベース接続を開きます。

conn.Open();

のオープンメソッド DbConnection のは抽象的なため、どのような例外をスローするかは、DbConnection から派生するプロバイダーによって決まります。また、抽象的な Open メソッドのドキュメントには、エラーが発生した場合に何が起こるかについてのガイダンスがありません。接続エラーが発生した場合、それを処理できないことはわかっています。呼び出し元がユーザーに UI を表示できる場所でエラーを発生させ、再試行させる必要があります。

後

public DbConnection GetConnection(String connectionName)
{
   //Get the connection string info from web.config
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];

   //documented to return null if it couldn't be found
    if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");

   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);

   //Undefined behaviour if GetFactory couldn't find a provider.
   //Defensive test for null factory anyway
   if (factory == null)
      throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");

   //Have the factory give us the right connection object
   DbConnection conn = factory.CreateConnection();

   //Undefined behaviour if CreateConnection failed
   //Defensive test for null connection anyway
   if (conn == null)
      throw new Exception("Could not obtain connection from factory");

   //Knowing the connection string, open the connection
   conn.ConnectionString = cs.ConnectionString;
   conn.Open()

   return conn;
}

まとめ

したがって、私の 4 行の関数は 12 行になり、ドキュメントの検索に 5 分かかりました。最終的に、メソッドが null を返すことが許可されているケースを 1 つ見つけました。しかし、実際には、アクセス違反例外（null参照でメソッドを呼び出そうとした場合）を例外に変換しただけでした。 無効な引数例外.

また、考えられるケースが 2 つあります。ヌルオブジェクトを返します。しかし、繰り返しますが、私は1つの例外を別の例外と交換しただけです。

良い面としては、2 つの問題が検出され、将来的に悪いことが起こるのではなく、例外メッセージで何が起こったかが説明されました。お金はここで止まります）

しかし、それだけの価値はあるでしょうか？これはやりすぎでしょうか？この防御的なプログラムは間違っているのでしょうか？

解決

構成を手動でチェックして例外をスローすることは、構成が欠落している場合にフレームワークに例外をスローさせることと同じです。いずれにせよ、フレームワークメソッド内で行われる前提条件チェックを複製しているだけであり、コードが冗長になり何のメリットもありません。(実際、あなたはそうかもしれません 削除する すべてを基本例外クラスとしてスローすることで情報を取得します。フレームワークによってスローされる例外は、通常、より具体的です。)

編集：この答えは多少物議を醸しているようですので、少し詳しく説明します。防御的なプログラミングとは、「予期せぬ事態に備える」（または「偏執的になる」）ことを意味し、そのための方法の 1 つは、前提条件のチェックを数多く行うことです。多くの場合、これは良い方法ですが、他の方法と同様に、コストと利点を比較検討する必要があります。

たとえば、「工場から接続を取得できませんでした」という例外をスローしても何のメリットもありません。なぜプロバイダーを取得できませんでした。プロバイダーが null の場合は、次の行で例外がスローされます。したがって、前提条件チェックのコスト (開発時間とコードの複雑さ) は正当化されません。

一方、接続文字列構成が存在することを確認するためのチェック 5月例外は開発者に問題の解決方法を伝えるのに役立つため、正当化されます。次の行で取得される null 例外は、欠落している接続文字列の名前を示していないため、前提条件チェックによって何らかの値が得られます。たとえば、コードがコンポーネントの一部である場合、コンポーネントのユーザーはコンポーネントに必要な構成がわからない可能性があるため、値は非常に大きくなります。

防御的プログラミングの別の解釈は、エラー状態を検出するだけでなく、発生する可能性のあるエラーや例外から回復することも試みるべきであるということです。一般的にこれが良い考えだとは思えません。

基本的には、可能な例外のみを処理する必要があります。する何かについて。いずれにしても回復できない例外は、トップレベルのハンドラーに上向きに渡す必要があります。Web アプリケーションでは、トップレベルのハンドラーはおそらく一般的なエラーページを表示するだけです。ただし、データベースがオフラインであるか、重要な構成が欠落している場合、ほとんどの場合、実際に行うべきことはそれほど多くありません。

この種の防御的なプログラミングが意味のある場合は、ユーザー入力を受け入れる場合であり、その入力がエラーにつながる可能性があります。たとえば、ユーザーが入力として URL を提供し、アプリケーションがその URL から何かをフェッチしようとする場合、URL が正しいかどうかを確認し、リクエストによって発生する可能性のある例外を処理することが非常に重要です。これにより、ユーザーに貴重なフィードバックを提供できます。

他のヒント

まあ、それはあなたの聴衆が誰であるかによって異なります。

他の多くの人が使用することが予想されるライブラリコードを作成していて、その使用方法について話してくれない場合は、やりすぎではありません。彼らはあなたの努力を高く評価するでしょう。

(とはいえ、そうする場合は、一部の例外はキャッチしたいが他の例外はキャッチしたくない人が簡単にできるように、単なる System.Exception よりも優れた例外を定義することをお勧めします。)

しかし、自分 (または自分とその友人) だけで使用する場合は、明らかにやりすぎであり、コードが読みにくくなり、最終的には害を及ぼす可能性があります。

私のチームにこのようなコーディングをさせられたらいいのにと思います。ほとんどの人は防御的なプログラミングの要点さえ理解していません。彼らが行う最善の方法は、メソッド全体を try catch ステートメントでラップし、すべての例外を汎用例外ブロックで処理させることです。

イアンさんには脱帽です。あなたのジレンマは理解できます。私自身も同じことを経験しました。しかし、あなたがやったことは、おそらく開発者が数時間キーボードを打ち続けていたのを助けたでしょう。

.net Framework API を使用するとき、そこに何を期待しているかを思い出してください。何が自然に見えるでしょうか？コードでも同じことを行います。

時間がかかることはわかっています。しかし、品質にはコストがかかります。

追伸：すべてのエラーを処理してカスタム例外をスローする必要はありません。あなたのメソッドは他の開発者によってのみ使用されることに注意してください。一般的なフレームワークの例外を自分で理解できる必要があります。それは苦労する価値がありません。

「前」の例には、明確かつ簡潔であるという特徴があります。

何か問題がある場合、最終的にフレームワークによって例外がスローされます。例外に対して何もできない場合は、例外を呼び出しスタックに伝播させたほうがよいでしょう。

ただし、フレームワークの奥深くで例外がスローされ、実際の問題が何であるかを明らかにしない場合があります。有効な接続文字列がないにもかかわらず、フレームワークが「無効な null の使用」などの例外をスローするという問題がある場合は、例外をキャッチして、より意味のあるメッセージを付けて再スローした方がよい場合があります。

実際のオブジェクトを操作する必要があるため、null オブジェクトを頻繁にチェックします。オブジェクトが空の場合、控えめに言っても、スローされる例外は斜めになります。ただし、null オブジェクトが発生することがわかっている場合にのみ、null オブジェクトをチェックします。一部のオブジェクトファクトリは null オブジェクトを返しません。代わりに例外がスローされ、このような場合には null のチェックは役に立ちません。

私はそのような null 参照チェックロジックを書きたくないと思います。少なくとも、あなたがそれを行った方法ではありません。

アプリケーション構成ファイルから構成設定を取得するプログラムは、起動時にそれらの設定をすべてチェックします。私は通常、設定を含む静的クラスを構築し、そのクラスのプロパティ (プロパティではなく) を参照します。 ConfigurationManager) アプリケーションの他の場所にあります。これには 2 つの理由があります。

まず、アプリケーションが適切に構成されていない場合、動作しません。これは、将来データベース接続を作成しようとした時点ではなく、プログラムが構成ファイルを読み取った時点で知りたいと思っています。

第 2 に、構成に依存するオブジェクトにとって、構成の有効性のチェックは実際には重要ではありません。すでにチェックを事前に実行している場合、コード全体のあらゆる場所にチェックを挿入するのは意味がありません。(確かにこれには例外があります。たとえば、プログラムの実行中に構成を変更し、その変更をプログラムの動作に反映できるようにする必要がある、長時間実行されるアプリケーションです。このようなプログラムでは、に行く必要があります ConfigurationManager 設定が必要なときはいつでも。)

私ならnull参照チェックはしません。 GetFactory そして CreateConnection 電話もします。そのコードを実行するためのテストケースをどのように作成しますか?これらのメソッドが null を返す方法を知らないため、それはできません。それが null であることさえ知りません。可能これらのメソッドが null を返すようにします。これで、1 つの問題が解決されました。プログラムでは次のような問題が発生する可能性があります。 NullReferenceException あなたが理解できない条件の下で - 別のより重要な条件で:同じ不可解な条件下では、プログラムはテストしていないコードを実行することになります。

私の経験則は次のとおりです。

スローされた例外のメッセージが発信者に関連しているかどうかをキャッチしないでください。

したがって、NullReferenceException には関連するメッセージがありません。null かどうかを確認し、より適切なメッセージを含む例外をスローします。ConfigurationErrorException は関連しているため、キャッチしません。

唯一の例外は、GetConnection の「コントラクト」が構成ファイル内の接続文字列を必ずしも取得しない場合です。

この場合、GetConnection には、接続を取得できなかったことを示すカスタム例外とのコントラクトが必要です (SHOULD)。カスタム例外で ConfigurationErrorException をラップできます。

もう 1 つの解決策は、GetConnection がスローできない (ただし null を返すことはできる) ことを指定し、クラスに「ExceptionHandler」を追加することです。

メソッドのドキュメントがありません。;-)

各メソッドには、いくつかの定義された入出力パラメータと、定義された結果の動作があります。あなたの場合、次のようなものです:「成功した場合は有効なオープン接続を返します。それ以外の場合は null を返します (または、必要に応じて XXXException をスローします)。この動作を念頭に置いて、どの程度防御的なプログラムを作成するかを決定できます。

メソッドが失敗した理由と何が失敗したかの詳細情報を公開する必要がある場合は、先ほどと同じように実行して、すべてをチェックしてキャッチし、適切な情報を返します。
ただし、オープン DBConnection に興味があるだけの場合、または単にヌル (またはユーザー定義の例外) 失敗した場合は、すべてを try/catch でラップして返すだけですヌルエラーの場合は (または何らかの例外)、それ以外の場合はオブジェクトです。

したがって、それはメソッドの動作と期待される出力に依存すると言えます。

一般に、データベース固有の例外は、(仮説の) ようなより一般的なものとしてキャッチして再スローする必要があります。 DataAccessFailure 例外。ほとんどの場合、高レベルのコードは、データベースからデータを読み取っていることを認識する必要はありません。

これらのエラーをすぐにトラップするもう 1 つの理由は、メッセージに「そのようなテーブルはありません:」のようなデータベースの詳細が含まれることが多いためです。ACCOUNTS_BLOCKED" または "ユーザーキーが無効です:234234」。これがエンドユーザーに伝播すると、いくつかの点で問題が発生します。

混乱する。
潜在的なセキュリティ侵害。
あなたの会社のイメージにとって恥ずかしいことです (クライアントが粗雑な文法でエラーメッセージを読んでいると想像してください)。

最初の試みとまったく同じようにコーディングしたでしょう。

ただし、その関数のユーザーは、USING ブロックを使用して接続オブジェクトを保護します。

他のバージョンのように例外を翻訳するのは好きではありません。なぜなら、例外が壊れた理由を見つけるのが非常に困難になるからです（データベースがダウンしましたか？）設定ファイルを読み取る権限がない場合など)。

アプリケーションに AppDomain.UnexpectedException をダンプするハンドラ exception.InnerException すべてのスタックトレースをある種のログファイルにチェーンし (またはさらに良いことに、ミニダンプをキャプチャし)、呼び出します。 Environment.FailFast.

この情報から、エラーチェックを追加してメソッドコードを複雑にする必要がなく、何が問題なのかを特定するのはかなり簡単です。

処理することが望ましいことに注意してください。 AppDomain.UnexpectedException そして電話する Environment.FailFast トップレベルを持つ代わりに try/catch (Exception x) なぜなら、後者の場合、問題の元の原因はさらなる例外によっておそらく不明瞭になるからです。

これは、例外をキャッチすると、開いているすべての finally ブロックが実行され、おそらくさらに多くの例外がスローされ、元の例外が隠蔽されます (さらに悪いことに、一部の状態を元に戻そうとファイルが削除され、間違ったファイルや重要なファイルが削除される可能性もあります)。たとえトップレベルであっても、処理方法がわからない無効なプログラム状態を示す例外をキャッチしてはなりません。 main 関数 try/catch ブロック。取り扱い AppDomain.UnexpectedException そして電話 Environment.FailFast 停止するため、別の（そしてより望ましい）魚の釜です。 finally プログラムを停止して、これ以上のダメージを与えずに役立つ情報をログに記録したい場合は、絶対に実行したくないでしょう。 finally ブロック。

OutOfMemoryExceptions を確認することを忘れないでください...知ってるでしょ かもしれない 起こる。

イアンの変更は私には賢明に思えます。

システムを使用していて、それが不適切に使用されている場合、その不正使用に関する最大限の情報が必要です。例えば。メソッドを呼び出す前に構成に値を挿入し忘れた場合は、KeyNotFoundException / NullReferenceException ではなく、エラーの詳細を示すメッセージを含む InvalidOperationException が必要です。

すべてはコンテキスト IMO に関するものです。私はこれまでに、かなり難解な例外メッセージをいくつか見たことがありますが、フレームワークからのデフォルトの例外がまったく問題ない場合もあります。

一般に、特に他の人によって頻繁に使用されるものを作成している場合や、通常はエラーの診断が難しいコールグラフの奥深くにあるものを作成している場合は、慎重にエラーを実行する方が良いと思います。

私は、コードやシステムの開発者として、それを単に使用している人よりも、障害を診断するのに有利な立場にあるということを常に覚えておくようにしています。場合によっては、数行のチェックコードとカスタム例外メッセージを追加することで、累積的に何時間ものデバッグ時間を節約できます (また、問題をデバッグするために他の人のマシンに引きずり込まれることがなくなり、自分自身の作業も楽になります)。

私の目には、あなたの「後」のサンプルはあまり防御的ではありません。なぜなら、防御とは自分のコントロール下にある部分をチェックすることであり、それが議論になるからです。 connectionName (null または空をチェックし、ArgumentNullException をスローします)。

防御的なプログラミングをすべて追加した後で、メソッドを分割してみてはいかがでしょうか?個別のメソッドを必要とする一連の個別のロジックブロックがあります。なぜ？なぜなら、一緒に属するロジックをカプセル化し、結果として得られるパブリックメソッドがそれらのブロックを正しい方法で接続するだけだからです。

このようなものです (SO エディターで編集されているため、構文/コンパイラーのチェックは行われません。コンパイルできない可能性があります ;-))

private string GetConnectionString(String connectionName)
{

   //Get the connection string info from web.config
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];

   //documented to return null if it couldn't be found
   if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
   return cs;
}

private DbProviderFactory GetFactory(String ProviderName)
{
   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = DbProviderFactories.GetFactory(ProviderName);

   //Undefined behaviour if GetFactory couldn't find a provider.
   //Defensive test for null factory anyway
   if (factory == null)
      throw new Exception("Could not obtain factory for provider \""+ProviderName+"\"");
   return factory;
}

public DbConnection GetConnection(String connectionName)
{
   //Get the connection string info from web.config
   ConnectionStringSettings cs = GetConnectionString(connectionName);

   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = GetFactory(cs.ProviderName);


   //Have the factory give us the right connection object
   DbConnection conn = factory.CreateConnection();

   //Undefined behaviour if CreateConnection failed
   //Defensive test for null connection anyway
   if (conn == null)
      throw new Exception("Could not obtain connection from factory");

   //Knowing the connection string, open the connection
   conn.ConnectionString = cs.ConnectionString;
   conn.Open()

   return conn;
}

追伸：これは完全なリファクタリングではなく、最初の 2 つのブロックのみを実行しました。

ライセンス： CC-BY-SA と帰属

所属していません StackOverflow