空のcatchブロックが悪い考えなのはなぜですか? [閉まっている]

StackOverflow https://stackoverflow.com/questions/1234343

  •  22-07-2019
  •  | 
  •  

質問

try-catchに関する質問を見たところです。 、空のcatchブロックは本当に悪い考えだと人々(Jon Skeetを含む)は言いますか?なんでこれ?空のキャッチが間違った設計決定ではない状況はありませんか?

たとえば、どこか(Webサービス、データベース)から追加の情報を取得したい場合がありますが、この情報を取得してもしなくてもかまいません。そのため、あなたはそれを取得しようとしますが、何かが起こっても大丈夫です、" catch(Exception ignored){}"を追加しますそれだけです

役に立ちましたか?

解決

通常は空のtry-catchは、エラー状態を静かに飲み込んでから実行を継続するため、悪い考えです。時々これは正しいことかもしれませんが、多くの場合、開発者が例外を見つけ、それについて何をすべきかを知らなかったため、空のキャッチを使用して問題を沈黙させた兆候です。

これは、エンジン警告灯の上に黒いテープを置くのと同じプログラミングです。

例外への対処方法は、使用しているソフトウェアのレイヤーによって異なると思います:熱帯雨林の例外

他のヒント

これらは悪いアイデアです。一般にこれは、障害(例外条件、より一般的には)が適切に応答なしで適切に満たされる、本当にまれな状態であるためです。さらに、空の catch ブロックは、例外エンジンを使用してプリエンプティブに実行する必要があることをエラーチェックする人々が使用する一般的なツールです。

それが常に悪いと言うのは真実ではありません...それはほとんど真実ではありません。エラーが発生したことを気にしないか、エラーの存在が何らかの形でそれについて何もできないことを示している場合があります(たとえば、以前のエラーをテキストログファイルに書き込み、 IOException が発生します。つまり、とにかく新しいエラーを書き出すことができませんでした。)

空のcatchブロックを使用するのは悪いプログラマーであり、彼が何をしているのかわからないと言う限り、私は物事を伸ばすつもりはありません...

必要に応じて、空のcatchブロックを使用します。私が使用しているライブラリのプログラマーは、彼が何をしているのか分からず、誰も必要としない状況でも例外をスローすることがあります。

たとえば、いくつかのhttpサーバーライブラリを考えてみてください。クライアントが切断され、 index.html を送信できなかったためにサーバーが例外をスローしても気にしません。

正当化できるまれな例があります。 Pythonでは、次のような構造がよく見られます:

try:
    result = foo()
except ValueError:
    result = None

だから、(アプリケーションによっては)大丈夫かもしれません:

result = bar()
if result == None:
    try:
        result = foo()
    except ValueError:
        pass # Python pass is equivalent to { } in curly-brace languages
 # Now result == None if bar() returned None *and* foo() failed

最近の.NETプロジェクトでは、特定のインターフェイスを実装するクラスを見つけるためにプラグインDLLを列挙するコードを作成する必要がありました。関連するコード(VB.NETではごめんなさい)は次のとおりです。

    For Each dllFile As String In dllFiles
        Try
            ' Try to load the DLL as a .NET Assembly
            Dim dll As Assembly = Assembly.LoadFile(dllFile)
            ' Loop through the classes in the DLL
            For Each cls As Type In dll.GetExportedTypes()
                ' Does this class implement the interface?
                If interfaceType.IsAssignableFrom(cls) Then

                    ' ... more code here ...

                End If
            Next
        Catch ex As Exception
            ' Unable to load the Assembly or enumerate types -- just ignore
        End Try
    Next

この場合でも、どこかに障害を記録することはおそらく改善されることを認めます。

通常、空のcatchブロックは、コーダーが実際に何をしているかわからないために配置されます。私の組織では、空のcatchブロックに、例外なしで何もしないことが良い考えである理由に関するコメントを含める必要があります。

関連するメモでは、ほとんどの人はtry {}ブロックの後にcatch {}またはfinally {}が続くことを知りません。1つだけが必要です。

例外は、本当に例外が発生した場合にのみスローされるべきです-何かが標準を超えて起こっています。空のcatchブロックは、基本的に「何か悪いことが起こっていますが、私は気にしません」と言っています。これは悪い考えです。

例外を処理したくない場合は、例外を処理できるコードに到達するまで上方に伝播させます。何も例外を処理できない場合は、アプリケーションを停止する必要があります。

特定の理由のみで発生する特定の例外タイプをキャッチしても大丈夫だと思います。それについて何もする必要はありません。

ただし、その場合でも、デバッグメッセージが順番に並んでいる可能性があります。

Josh Bloch あたり-項目65: click / com / 0321356683 "rel =" noreferrer ">有効なJava

  1. 空のcatchブロックは例外の目的を無効にします
  2. 少なくとも、catchブロックには、例外を無視することが適切である理由を説明するコメントを含める必要があります。

空のcatchブロックは、基本的に"スローされたエラーを知りたくないので、無視するつもりです"

と言っています。

VB6の On Error Resume Next と似ていますが、例外がスローされた後のtryブロック内のすべてのものがスキップされます。

何かが壊れたときに助けにはなりません。

これは、「プログラムフローの制御に例外を使用しないでください」、および「例外的な状況にのみ例外を使用する」と密接に関係しています。これらが完了している場合、問題が発生した場合にのみ例外が発生するはずです。そして、問題がある場合、黙って失敗したくありません。問題を処理する必要のないまれな異常では、異常が異常でなくなった場合に備えて、少なくとも例外をログに記録する必要があります。失敗よりも悪いのは、黙って失敗することだけです。

完全に空のcatchブロックは、例外を無視することがコードの意図された動作であると推測する方法がないため、悪い考えだと思います。例外を飲み込み、場合によってはfalseまたはnullまたはその他の値を返すことは必ずしも悪いことではありません。 .netフレームワークには多くの「試用」があります。このように動作するメソッド。例外を飲み込む場合の経験則として、アプリケーションがロギングをサポートしている場合はコメントとログステートメントを追加します。

例外がスローされると 例外が表示されないため、黙って失敗するのが最悪の選択肢です。誤った動作が発生し、どこで発生しているのか見当がつきません。少なくともそこにログメッセージを入れてください!たとえそれが「決して起こらない」ものであっても!

空のcatchブロックは、プログラマーが例外の処理方法を知らないことを示しています。例外がバブリングし、別のtryブロックによって正しく処理される可能性を抑制しています。キャッチしている例外を除いて、常に何かを試してみてください。

空のcatchステートメントで最も厄介なのは、他のプログラマーがやったときです。つまり、他の誰かからコードをデバッグする必要がある場合、空のcatchステートメントがあると、そのような作業が必要以上に難しくなります。 IMHO catchステートメントは、常に何らかのエラーメッセージを表示する必要があります-エラーが処理されない場合でも、少なくともそれを検出する必要があります(alt。onはデバッグモードのみ)

可能性のある例外は黙ってすべてに渡されるため、おそらく正しいことではありません。予期している特定の例外がある場合は、それをテストし、例外でない場合は再スローする必要があります。

try
{
    // Do some processing.
}
catch (FileNotFound fnf)
{
    HandleFileNotFound(fnf);
}
catch (Exception e)
{
    if (!IsGenericButExpected(e))
        throw;
}

public bool IsGenericButExpected(Exception exception)
{
    var expected = false;
    if (exception.Message == "some expected message")
    {
        // Handle gracefully ... ie. log or something.
        expected = true;
    }

    return expected;
}

通常、実際に処理できる例外のみをキャッチする必要があります。これは、例外をキャッチするときに可能な限り具体的にすることを意味します。すべての例外をキャッチすることはめったに良いアイデアではありませんし、すべての例外を無視することはほとんど常に非常に悪いアイデアです。

空のcatchブロックに意味のある目的があるいくつかの例しか考えられません。特定の例外が何であれ、キャッチしているのが「処理」されている場合アクションを再試行するだけで、catchブロックで何もする必要がなくなります。ただし、例外が発生したという事実をログに記録することをお勧めします。

別の例:CLR 2.0は、ファイナライザースレッドの未処理の例外の処理方法を変更しました。 2.0より前のプロセスは、このシナリオに耐えることができました。現在のCLRでは、ファイナライザースレッドで未処理の例外が発生した場合、プロセスは終了します。

ファイナライザが本当に必要な場合にのみ、ファイナライザを実装する必要があり、その場合でもファイナライザで少しだけ実行する必要があることに留意してください。しかし、ファイナライザが実行しなければならない処理が例外をスローする可能性がある場合は、2つの悪のどちらか小さい方を選択する必要があります。未処理の例外のためにアプリケーションをシャットダウンしますか?または、多かれ少なかれ未定義の状態で続行しますか?少なくとも理論的には、後者は、いくつかのケースでは2つの悪の小さい方かもしれません。これらの場合、空のcatchブロックはプロセスの終了を妨げます。

たとえば、どこか(Webサービス、データベース)から追加情報を取得したい場合がありますが、この情報を取得するかどうかは本当に気にしません。そのため、あなたはそれを取得しようとしますが、何かが起こっても大丈夫です、" catch(Exception ignored){}"を追加しますそれがすべてです

それで、あなたの例で言えば、あなたは all 例外をキャッチして無視しているので、その場合は悪い考えです。 EInfoFromIrrelevantSourceNotAvailable のみをキャッチして無視した場合、それは問題ありませんが、そうではありません。また、 ENetworkIsDown も無視しますが、これは重要である場合と重要でない場合があります。あなたは ENetworkCardHasMelted EFPUHasDecidedThatOnePlusOneIsSeventeen を無視していますが、これらはほぼ確実に重要です。

重要でないことがわかっている特定のタイプの例外のみをキャッチ(および無視)するように設定されている場合、空のcatchブロックは問題になりません。 all 例外を抑制して黙って無視することをお勧めします。まず例外を調べて、それらが予期される/正常/無関係であるかどうかを確認しますが、非常にまれです。

これらを使用する場合もありますが、非常にまれです。使用する可能性のある状況は次のとおりです。

  • 例外ログ。コンテキストに応じて、未処理の例外またはメッセージを代わりに投稿することができます。

  • 動作自体が問題を実証するレンダリングやサウンド処理、リストボックスコールバックなどの技術的な状況をループし、例外をスローすると邪魔になり、例外をログに記録するのはおそらく1000の「XXXに失敗しました」メッセージ。

  • できないプログラムですが、少なくとも何かを記録する必要があります。

ほとんどのwinformsアプリケーションでは、すべてのユーザー入力に対して1つのtryステートメントがあれば十分であることがわかりました。私は次のメソッドを使用します:(AlertBoxは単なるクイックMessageBox.Showラッパーです)

  public static bool TryAction(Action pAction)
  {
     try { pAction(); return true; }
     catch (Exception exception)
     {
        LogException(exception);
        return false;
     }
  }

  public static bool TryActionQuietly(Action pAction)
  {
     try { pAction(); return true; }
     catch(Exception exception)
     {
        LogExceptionQuietly(exception);
        return false;
     }
  }

  public static void LogException(Exception pException)
  {
     try
     {
        AlertBox(pException, true);
        LogExceptionQuietly(pException);
     }
     catch { }
  }

  public static void LogExceptionQuietly(Exception pException)
  {
     try { Debug.WriteLine("Exception: {0}", pException.Message); } catch { }
  }

すべてのイベントハンドラーは次のようなことができます:

  private void mCloseToolStripMenuItem_Click(object pSender, EventArgs pEventArgs)
  {
     EditorDefines.TryAction(Dispose);
  }

または

  private void MainForm_Paint(object pSender, PaintEventArgs pEventArgs)
  {
     EditorDefines.TryActionQuietly(() => Render(pEventArgs));
  }

理論的には、TryActionSilentlyを使用することができます。これは、例外が無限の量のメッセージを生成しないように、呼び出しのレンダリングに適している場合があります。

空のcatchブロックを使用しないでください。知っている間違いを隠すようなものです。少なくとも、時間に追われた場合に後で確認するために、ログファイルに例外を書き出す必要があります。

catchブロックで何をすべきかわからない場合は、この例外をログに記録できますが、空白のままにしないでください。

        try
        {
            string a = "125";
            int b = int.Parse(a);
        }
        catch (Exception ex)
        {
            Log.LogError(ex);
        }
ライセンス: CC-BY-SA帰属
所属していません StackOverflow
scroll top