他人が書いたコードを読んでいる時に 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