catchブロックのないfinallyブロックはjavaアンチパターンですか?
-
03-07-2019 - |
質問
次のようなコードのトラブルシューティングで、かなり苦痛なトラブルシューティングの経験がありました。
try {
doSomeStuff()
doMore()
} finally {
doSomeOtherStuff()
}
doSomeStuff()が例外をスローし、その結果doSomeOtherStuff()も例外をスローするため、問題のトラブルシューティングは困難でした。 2番目の例外(finallyブロックによってスローされる)は私のコードまでスローされましたが、最初の例外(doSomeStuff()からスローされる)に対するハンドルはありませんでした。これは問題の本当の根本原因でした。
コードがこれを代わりに言っていた場合、問題はすぐに明らかになります:
try {
doSomeStuff()
doMore()
} catch (Exception e) {
log.error(e);
} finally {
doSomeOtherStuff()
}
だから、私の質問はこれです:
catchブロックなしで使用されるfinallyブロックは、有名なJavaアンチパターンですか? (それは確かによく知られているアンチパターンの「すぐに見かけ上は見えないサブクラスのようです" Do n't gobble not exception!」
解決
通常、いいえ、これはアンチパターンではありません。 finallyブロックのポイントは、例外がスローされるかどうかに関係なく、内容が確実にクリーンアップされるようにすることです。例外処理のポイントは、対処できない場合は、例外処理が提供する比較的クリーンな帯域外シグナリングを介して、処理できる人にバブルさせることです。例外がスローされた場合に確実にクリーンアップする必要があるが、現在のスコープで例外を適切に処理できない場合、これはまさに正しいことです。 finallyブロックがスローされないようにするために、もう少し注意が必要な場合があります。
他のヒント
実際の「アンチパターン」はここでは、キャッチすることなく、スローできる finally
ブロックで何かを実行しています。
まったくありません。
問題は、finally内のコードです。
最終的には常に実行され、例外をスローする可能性のあるものをそこに置くことは(目撃したように)ただリスクがあることを思い出してください。
finallyとno catchを試してみても、間違いはありません。以下を考慮してください:
InputStream in = null;
try {
in = new FileInputStream("file.txt");
// Do something that causes an IOException to be thrown
} finally {
if (in != null) {
try {
in.close();
} catch (IOException e) {
// Nothing we can do.
}
}
}
例外がスローされ、このコードがそれを処理する方法を知らない場合、例外は呼び出しスタックを呼び出し元にバブルする必要があります。この場合でも、ストリームをクリーンアップしたいので、キャッチせずにtryブロックを使用するのが理にかなっていると思います。
これはアンチパターンとはほど遠いものであり、メソッドの実行中に取得したリソースの割り当てを解除することが重要な場合に非常に頻繁に行うことです。
ファイルハンドル(書き込み用)を扱うときに行うことの1つは、例外をスローしないIOUtils.closeQuietlyメソッドを使用してストリームを閉じる前にストリームをフラッシュすることです。
OutputStream os = null;
OutputStreamWriter wos = null;
try {
os = new FileOutputStream(...);
wos = new OutputStreamWriter(os);
// Lots of code
wos.flush();
os.flush();
finally {
IOUtils.closeQuietly(wos);
IOUtils.closeQuietly(os);
}
次の理由から、そのようにするのが好きです:
- ファイルを閉じるときに例外を無視することは完全に安全ではありません-ファイルにまだ書き込まれていないバイトがある場合、ファイルは呼び出し側が期待する状態にない可能性があります。
- したがって、flush()メソッド中に例外が発生した場合、それは呼び出し元に伝播されますが、すべてのファイルが閉じられていることを確認します。メソッドIOUtils.closeQuietly(...)は、対応するtry ... catch ... ignore meブロックよりも冗長です;
- 複数の出力ストリームを使用する場合、flush()メソッドの順序が重要です。コンストラクターとして他のストリームを渡すことによって作成されたストリームは、最初にフラッシュする必要があります。同じことはclose()メソッドにも有効ですが、flush()は私の意見ではより明確です。
catchブロックのないtryブロックはアンチパターンだと思います。 「キャッチのない最終的にはありません」と言うは、「キャッチせずに試してはいけない」のサブセットです。
次の形式でtry / finallyを使用します:
try{
Connection connection = ConnectionManager.openConnection();
try{
//work with the connection;
}finally{
if(connection != null){
connection.close();
}
}
}catch(ConnectionException connectionException){
//handle connection exception;
}
try / catch / finally(+最終的にネストされたtry / catch)よりもこれを好みます。 もっと簡潔で、catch(Exception)を複製しないと思います。
try {
doSomeStuff()
doMore()
} catch (Exception e) {
log.error(e);
} finally {
doSomeOtherStuff()
}
それをしないでください...バグを隠しただけです(正確には隠していません...しかし、それらを処理するのを難しくしました。およびArrayIndexOutOfBounds)。
通常、キャッチする必要がある例外(チェック済み例外)をキャッチし、テスト時に他の例外に対処します。 RuntimeExceptionsは、プログラマーのエラーに使用するように設計されています。プログラマーのエラーは、適切にデバッグされたプログラムでは発生しないはずのことです。
私の意見では、 catch
を伴う finally
が何らかの問題を示しているということです。リソースのイディオムは非常に簡単です:
acquire
try {
use
} finally {
release
}
Javaでは、ほとんどどこからでも例外が発生する可能性があります。多くの場合、取得はチェック済み例外をスローします。これを処理する賢明な方法は、どれくらいの量をキャッチするかです。恐ろしいnullチェックを試みないでください。
もしあなたが本当にアナルになるつもりなら、例外には暗黙の優先順位があることに注意すべきです。たとえば、ThreadDeathは、取得/使用/リリースに関係なく、すべてを壊すべきです。これらの優先順位を正しく処理するのは見苦しいです。
そのため、リソース処理を抽象化して、Execute Aroundイディオムを使用します。
Try / Finallyは、リソースを適切に解放する方法です。 tryブロックに入る前に取得されたリソースまたは状態にのみ作用するため、finallyブロックのコードは決してスローすべきではありません。
余談ですが、log4Jはアンチパターンであると思われます。
実行中のプログラムを検査したい場合は、適切な検査ツールを使用します(つまり、デバッガー、IDE、または極端な意味でバイトコードウィーバーを使用しますが、数行ごとにステートメントを記録しないでください!)。
2つの例では、最初の例が正しいように見えます。 2番目のものにはロガーコードが含まれており、バグが発生します。 2番目の例では、最初の2つのステートメントによって例外がスローされた場合に例外を抑制します(つまり、キャッチしてログに記録しますが、再スローはしません。これはlog4jの使用で非常に一般的であり、アプリケーション設計の実際の問題です。変更を加えると、例外が発生していないかのように基本的に前進するので、システムが処理するのが非常に困難な方法でプログラムを失敗させます(VB Basic on Error Resume Next Constructなど)。
try-finally
は、メソッドに複数の return
ステートメントがある場合に、コピーアンドペーストコードを減らすのに役立ちます。次の例を検討してください(Android Java):
boolean doSomethingIfTableNotEmpty(SQLiteDatabase db) {
Cursor cursor = db.rawQuery("SELECT * FROM table", null);
if (cursor != null) {
try {
if (cursor.getCount() == 0) {
return false;
}
} finally {
// this will get executed even if return was executed above
cursor.close();
}
}
// database had rows, so do something...
return true;
}
finally
節がなかった場合、 cursor.close()
を2回書く必要があるかもしれません: return false
の直前と後周囲の if
句。
キャッチなしで試すのはアンチパターンだと思います。 try / catchを使用して例外的な条件(ファイルIOエラー、ソケットタイムアウトなど)を処理することはアンチパターンではありません。
try / finallyをクリーンアップに使用している場合は、代わりにusingブロックを検討してください。