他人が書いたコードを読んでいる時に 10 億回くらい思うこと
他人が書いたコードを読んでいる時に 10 億回くらい思うことを書いておこうと思う。
using
してるのに finally
で Dispose()
したり Close()
しちゃう
何もわからないのだが、よく using
で囲っているのにその中の try
... finally
文の finally
節で Dispose()
したり Close()
してしまっているコードを見る。
以下はとても典型的なコードで、もう一つ問題があるのだが、これは次で解説する。
dim result = new DataTable() using reader = command.ExecuteReader() try result.Load( reader ) finally reader.Dispose() end try end using return result
ここで、reader
は明らかに IDisposable
を実装しているのですごく単純に考えると end using
のところで自動的に Dispose()
されるので、finally
の reader.Dispose()
は必要ない。
多分、このコードを書くような人は、古 Java 話者の人か古 Java を少し話せる人で、using
をただのおまじないか何かと思っているのだろう。
私が書き直すなら以下のようになる:
dim result = new DataTable() using reader = command.ExecuteReader() try result.Load( reader ) catch ex as Exception Trace.TraceError( ex.ToString() ) end try end using return result
finally
の替わりに catch
節にして、例外を出力ウィンドウに出力するようにした。
スコープ引き伸ばし症
先程書き直したコードだが、まだ改善するべき箇所がある。
それは result
の宣言を using
スコープの外に出している点である。
dim result = new DataTable() using reader = command.ExecuteReader() try result.Load( reader ) catch ex as Exception Trace.TraceError( ex.ToString() ) end try end using return result
スコープの範囲を最小限にする原則からすれば、result
の宣言は try
節の中で良いし、result
を return
するところも try
節の中でいい。
そのことを踏まえて書き直すと以下のようになる:
using reader = command.ExecuteReader() try dim result = new DataTable() result.Load( reader ) return result catch ex as Exception Trace.TraceError( ex.ToString() ) end try end using