- PR -

COM参照を確実に解放するコードの可読性をあげたい

投稿者投稿内容
ぼのぼの
ぬし
会議室デビュー日: 2004/09/16
投稿数: 544
投稿日時: 2006-03-15 13:21
こんにちは。
今、VB.NETのプログラムからExcelを操作する処理を、
じゃんぬねっとさんのサイトを参考に作っているのですが、
あまりに可読性が悪いので(^^;、ちょっと書き方を工夫しようかと考えています。

ポイントになるのは、例外発生時の挙動だと思います。
そこで、まず下記のようなコードを書いてテストしてみました。

コード:


Private Sub Button1_Click( _
ByVal sender As System.Object, ByVal e As System.EventArgs) _
Handles Button1.Click
Try
TestMethod()
Catch ex As Exception
Me.TextBox1.Text = ex.ToString()
End Try
End Sub

Private Sub TestMethod()
Try
Throw New Exception("A")
Finally
Try
Throw New Exception("B")
Finally
Try
Throw New Exception("C")
Finally
Throw New Exception("D")
End Try
End Try
End Try
End Sub


動かしてみると、TextBox1に表示されるのはDの情報でした。
つまり、最後に発生した例外のみが上位に通知されたことになります。

単純に考えると、ネストを浅くして同じ動作をさせたければ、
以下のようなコードを書けば良いのかな、と考えました。

コード:


Private Sub TestMethod2()
Dim lastEx As Exception = Nothing
Try
Throw New Exception("A")
Catch ex As Exception
lastEx = ex
End Try
Try
Throw New Exception("B")
Catch ex As Exception
lastEx = ex
End Try
Try
Throw New Exception("C")
Catch ex As Exception
lastEx = ex
End Try
Try
Throw New Exception("D")
Catch ex As Exception
lastEx = ex
End Try
If Not lastEx Is Nothing Then Throw lastEx
End Sub


ここから先は設計思想の問題なのですが、
この例のように複数のFinallyブロックで例外が発生し得る処理を、
Catchを使って書き換える場合、皆さんならどのように考えますか?
  • 元のコードと同じく、最後に発生した例外のみ通知できれば良い
  • 逆に、最初に発生した例外のみ通知した方が良い
  • いやいや、ArrayListやCollectionを使って発生した全ての例外を通知すべきだ

よろしければご意見お聞かせください。

#スレタイの漢字を修正

[ メッセージ編集済み 編集者: ぼのぼの 編集日時 2006-03-15 14:03 ]
じゃんぬねっと
ぬし
会議室デビュー日: 2004/12/22
投稿数: 7811
お住まい・勤務地: 愛知県名古屋市
投稿日時: 2006-03-15 13:57
件名 -> s/開放/解放/g

引用:

ぼのぼのさんの書き込み (2006-03-15 13:21) より:

ここから先は設計思想の問題なのですが、
この例のように複数のFinallyブロックで例外が発生し得る処理を、
Catchを使って書き換える場合、皆さんならどのように考えますか?


すみません、ちょっと方向性が見えませんでした。
結果的にどうしたいんでしょうか?

Finally パターンによる実装は解放漏れを確実に防ぐ最も良い手段です。
可読性が問題であるにしても、何故例外処理のことだけを考えるのでしょうか?

以前、以下のスレッドでもいくつか意見を頂戴したので参考リンクしておきます。

  ASP.NET からの Excel 作成でエラー

_________________
C# と VB.NET の入門サイト
じゃんぬねっと日誌
ぼのぼの
ぬし
会議室デビュー日: 2004/09/16
投稿数: 544
投稿日時: 2006-03-15 14:44
あっ、過去スレあったんですね…失礼しました(^^;

引用:

じゃんぬねっとさんの書き込み (2006-03-15 13:57) より:

すみません、ちょっと方向性が見えませんでした。
結果的にどうしたいんでしょうか?



リンク先の過去スレにちょうどいいのがあったんでいくつか引用します。

引用:

Jittaさんの書き込み (2006-02-21 22:03) より:
インデントが深くなるのが嫌で、「まとめて宣言、まとめて解放」をするようになりました。。。



引用:

じゃんぬねっとさんの書き込み (2006-02-22 00:06) より:
COM Interop の場合、それでは安全とは言えないですね。
せめてまとめての解放に Finally の機構が必要になります。

と言いつつも、私も昔はまとめてやっていました。



引用:

かるあさんの書き込み (2006-02-22 01:22) より:
開放できないと困り物なのでやらなければいけないのはわかりますが
ロジックが埋もれてしまいそうですね・・・



引用:

じゃんぬねっとさんの書き込み (2006-02-22 02:01) より:
引用:

ジブさんの書き込み (2006-02-22 01:33) より:

こう書いて等価のようにも思えます


いいえ、等価ではありません。

Dispose メソッドの場合は、例外が発生しないことが保証されていますが、
ReleaseComObject メソッドは保証されていません。



引用:

じゃんぬねっとさんの書き込み (2006-02-26 21:31) より:
引用:

まぁ、Windows アプリケーションの場合、例外でアプリケーションが落ちれば解放もされますが、ASP.NET の場合は、ねぇ。。。


COM の場合は勝手には解放されませんよー。



この一連の流れが、私が結果的にどうしたいかをすごく良く表してます。
「インデントが深くなるのが嫌」これは「可読性が悪い」ということで、
「可読性が悪い」ということは「保守性が悪い」ということで。
例えば、後から扱うシートやセルが増えてソースに手を入れることになった場合、
どこをどう直していいのか混乱する(本人でさえ、他人がやるならなおさら)。

なので、多少なりとも「保守しやすいコードにしたい」と。

しかし、それ以降の一連の流れで語られている通り、
.NETでCOM参照が解放されないのは致命的。つまり、コードを変えるにしても
全てのReleaseComObject()が確実に実行されるようにしなければならない

保守しやすいコードを考えるに当たり、まずはインデントを浅くする工夫から始めるのが
てっとりばやいと考えました。
通常、インデントを浅くしたければ、「一部のサブルーチン化」か「アルゴリズムの見直し」になります。
このどちらかの方法で、インデントを浅くしつつ、元のコードと全く同じ挙動をするような処理を考えてたわけです。
ここでいう全く同じ挙動とは、
(1)メソッドの入力(引数)が同じで、
(2)メソッドの出力(戻り値)が同じで、
(3)エラー発生時にthrowする例外が同じで、
(4)処理結果(Excelファイルの操作と全てのCOM参照の確実な解放)が同じ
という意味です。

それで、(3)を考えてた過程で、最初の書き込みの問題が出てきまして。
異常系処理である(3)は、厳密に元の挙動と揃える必要ないかな?
違うくするなら、どうすると使う側として一番都合がいいかな?
と思い、スレッドを立てた次第でした。

過程がすっぽり抜けてて、わかりづらかったですね。すみません。。。
じゃんぬねっと
ぬし
会議室デビュー日: 2004/12/22
投稿数: 7811
お住まい・勤務地: 愛知県名古屋市
投稿日時: 2006-03-15 14:58
引用:

ぼのぼのさんの書き込み (2006-03-15 14:44) より:

あっ、過去スレあったんですね…失礼しました(^^;


いえいえ、件名とは別の方向へ進んだスレッドなので見つかるわけがないですよね。(^^)

引用:

Dispose メソッドの場合は、例外が発生しないことが保証されていますが、
ReleaseComObject メソッドは保証されていません。


これは、向こうのスレッドでも注意されたようにウソでした。(;^-^)
複数回呼び出しによる呼び出しが保証されているという意味でした。

引用:

「インデントが深くなるのが嫌」これは「可読性が悪い」ということで、
「可読性が悪い」ということは「保守性が悪い」ということで。


確かに可読性はかなり悪いです。(最も安全ですが)
ですので、いつも言い訳のように「仕方がない」ことを書いています。(^^)

COM の場合は暗黙の参照による参照カウントのインクリメントを防ぐことが要になるので、
どうしてもインデントされた構造が必要なのですが、実際の業務では解放が最後になってしまうことが多いですね。
だったら... と思うことは確かにあります。

引用:

ここでいう全く同じ挙動とは、
(1)メソッドの入力(引数)が同じで、
(2)メソッドの出力(戻り値)が同じで、


このメソッドってどれを指していますか?

引用:

(3)エラー発生時にthrowする例外が同じで、
(4)処理結果(Excelファイルの操作と全てのCOM参照の確実な解放)が同じ

それで、(3)を考えてた過程で、最初の書き込みの問題が出てきまして。
異常系処理である(3)は、厳密に元の挙動と揃える必要ないかな?
違うくするなら、どうすると使う側として一番都合がいいかな?と思い、スレッドを立てた次第でした。


なるほど、わかりました。
向こうのスレッドでも書いたのですが、params (ParamArray) じゃ事足りないでしょうか?
全くの等価は無理ですが、このレベルであればある程度は等価にできるかもしれません。

# でも参照渡しができないので精神衛生上、良くないのかも...

_________________
C# と VB.NET の入門サイト
じゃんぬねっと日誌
囚人
ぬし
会議室デビュー日: 2005/08/13
投稿数: 1019
投稿日時: 2006-03-15 15:05
COM Interop ってあんまり使ったこと無いから、よくわからんのだけども、COM ラッパーをラップする、ってのどうなんですかね。
COM ラッパーラッパーの Dispose() とファイナライザで参照カウントを減らしてやる。

_________________
囚人のジレンマな日々
じゃんぬねっと
ぬし
会議室デビュー日: 2004/12/22
投稿数: 7811
お住まい・勤務地: 愛知県名古屋市
投稿日時: 2006-03-15 15:22
引用:

囚人さんの書き込み (2006-03-15 15:05) より:

COM Interop ってあんまり使ったこと無いから、よくわからんのだけども、COM ラッパーをラップする、ってのどうなんですかね。
COM ラッパーラッパーの Dispose() とファイナライザで参照カウントを減らしてやる。


挑戦するとよ〜くわかるんですが、参照される "機会" が増えるので余計に厄介になるんですよ。
IDisposable.Dispose みたいな実装については、かの中博俊さんも嘆いてましたね。

完成しても余計に精神衛生上よろしくなかったりします...

_________________
C# と VB.NET の入門サイト
じゃんぬねっと日誌
ぼのぼの
ぬし
会議室デビュー日: 2004/09/16
投稿数: 544
投稿日時: 2006-03-15 16:22
引用:

じゃんぬねっとさんの書き込み (2006-03-15 14:58) より:
このメソッドってどれを指していますか?


すみません、実はまだ作ってないんです(^^;
正確にいうと、「本物」は都合によりお見せできないのですが、
お見せできるよう加工したものは出来次第貼ります。
とりあえず、じゃんぬさんのサイトにあるサンプルコードの外側を
Public Shared Sub SampleMethod()で囲んだようなものを想像してください。

<編集>
うわ、書いてるうちに書き込みが…(笑)
じゃんぬさん、サンプル書くの速いっす(^^;
しかしサンプルとしてはちょうどいいので使わせて頂きますm(_ _)m
</編集>
引用:

囚人さんの書き込み (2006-03-15 15:05) より:
COM Interop ってあんまり使ったこと無いから、よくわからんのだけども、COM ラッパーをラップする、ってのどうなんですかね。
COM ラッパーラッパーの Dispose() とファイナライザで参照カウントを減らしてやる。


未だにVS2003使ってる&Office2003も持ってないので、想像でしかないのですが、
VSTOを導入すれば似たようなことをやってくれるんでしょうか?
一瞬「作ろうか」と思ってしまいましたが、じゃんぬさんの書き込みを見て
「やめよう」と思ってしまいました(笑)

[ メッセージ編集済み 編集者: ぼのぼの 編集日時 2006-03-15 16:46 ]
じゃんぬねっと
ぬし
会議室デビュー日: 2004/12/22
投稿数: 7811
お住まい・勤務地: 愛知県名古屋市
投稿日時: 2006-03-15 16:35
引用:

私の書き込み (2006-03-15 14:58) より:

向こうのスレッドでも書いたのですが、params (ParamArray) じゃ事足りないでしょうか?
全くの等価は無理ですが、このレベルであればある程度は等価にできるかもしれません。


Excel を起動 -> 新しい WorkBook を追加 -> セルに値をセットして保存して終了という流れの場合。

まずは、定番の安全だけど可読性の悪いコード。

コード:


Private Shared Sub MosaMosaAA()
Dim xlApplication As Excel.Application

Try
xlApplication = New Excel.Application()
xlApplication.Visible = True

Dim xlWorkBooks As Excel.Workbooks

Try
xlWorkBooks = xlApplication.Workbooks
Dim xlWorkBook As Excel.Workbook

Try
xlWorkBook = xlWorkBooks.Add()
Dim xlWorkSheets As Excel.Sheets

Try
xlWorkSheets = xlWorkBook.Worksheets
Dim xlSheet1 As Excel.Worksheet

Try
xlSheet1 = DirectCast(xlWorkSheets("Sheet1"), Excel.Worksheet)
Dim xlCells As Excel.Range

Try
xlCells = xlSheet1.Cells
Dim xlRange As Excel.Range

Try
xlRange = DirectCast(xlCells(1, 1), Excel.Range)
xlRange.Value2 = "COM Interop is hated!"
Finally
If Not xlRange Is Nothing Then
System.Runtime.InteropServices.Marshal.ReleaseComObject(xlRange)
End If
End Try
Finally
If Not xlCells Is Nothing Then
System.Runtime.InteropServices.Marshal.ReleaseComObject(xlCells)
End If
End Try
Finally
If Not xlSheet1 Is Nothing Then
System.Runtime.InteropServices.Marshal.ReleaseComObject(xlSheet1)
End If
End Try
Finally
If Not xlWorkSheets Is Nothing Then
System.Runtime.InteropServices.Marshal.ReleaseComObject(xlWorkSheets)
End If
End Try

xlApplication.DisplayAlerts = False
xlWorkBook.SaveAs("C:\Book1.xls")
Finally
If Not xlWorkBook Is Nothing Then
Try
xlWorkBook.Close()
Finally
System.Runtime.InteropServices.Marshal.ReleaseComObject(xlWorkBook)
End Try
End If
End Try
Finally
If Not xlWorkBooks Is Nothing Then
Try
xlWorkBooks.Close()
Finally
System.Runtime.InteropServices.Marshal.ReleaseComObject(xlWorkBooks)
End Try
End If
End Try
Finally
If Not xlApplication Is Nothing Then
Try
xlApplication.Quit()
Finally
System.Runtime.InteropServices.Marshal.ReleaseComObject(xlApplication)
End Try
End If
End Try
End Sub


これが、こうなります。

コード:


Private Shared Sub HogeHogeFoobar()
Dim xlApplication As Excel.Application
Dim xlWorkBooks As Excel.Workbooks
Dim xlWorkBook As Excel.Workbook
Dim xlWorkSheets As Excel.Sheets
Dim xlSheet1 As Excel.Worksheet
Dim xlCells As Excel.Range
Dim xlRange As Excel.Range

Try
xlApplication = New Excel.Application()
xlApplication.Visible = True
xlWorkBooks = xlApplication.Workbooks
xlWorkBook = xlWorkBooks.Add()
xlWorkSheets = xlWorkBook.Worksheets
xlSheet1 = DirectCast(xlWorkSheets("Sheet1"), Excel.Worksheet)
xlCells = xlSheet1.Cells
xlRange = DirectCast(xlCells(1, 1), Excel.Range)
xlRange.Value2 = "COM Interop is hated!"
xlApplication.DisplayAlerts = False
xlWorkBook.SaveAs("C:\Book1.xls")
Finally
ReleaseAnyComObject(True, xlRange, xlCells, xlSheet1, xlWorkSheets, xlWorkBook, xlWorkBooks, xlApplication)
End Try
End Sub


うーん、危険な香りはしますがスッキリしました。
さて、次に ReleaseAnyComObject メソッドの実装です。

コード:


#Region " ReleaseAnyComObject メソッド (+1 のオーバーロード) "

Private Shared Overloads Sub ReleaseAnyComObject(ParamArray comObjects As Object())
If comObjects Is Nothing Then
Return
End If

' MEMO : For Each は使わない方が良さげ
For i As Integer = 0 To comObjects.Length - 1
Try
System.Runtime.InteropServices.Marshal.ReleaseComObject(comObjects(i))
Catch ex As Exception
' TODO : 最初/最後の例外を格納しておくなり、全部リストに追加するなり
' ココがぼのぼのさんの問題視している部分?
End Try
Next i
End Sub

Private Shared Overloads Sub ReleaseAnyComObject(ByVal isQuit As Boolean, ParamArray comObjects As Object())
If Not isQuit Then
ReleaseAnyComObject(comObjects)
Return
End If

If comObjects Is Nothing Then
Return
End If

' MEMO : For Each は使わない方が良さそう
For i As Integer = 0 To comObjects.Length - 1
Try
' MEMO : ここが結構怖い
If TypeOf comObjects(i) Is Excel.Workbook Then
DirectCast(comObjects(i), Excel.Workbook).Close()
ElseIf TypeOf comObjects(i) Is Excel.Workbooks Then
DirectCast(comObjects(i), Excel.Workbooks).Close()
ElseIf TypeOf comObjects(i) Is Excel.Application Then
DirectCast(comObjects(i), Excel.Application).Quit()
End If
Catch ex1 As Exception
' TODO : 最初/最後の例外を格納しておくなり、全部リストに追加するなり
' ココがぼのぼのさんの問題視している部分?
Finally
Try
System.Runtime.InteropServices.Marshal.ReleaseComObject(comObjects(i))
Catch ex2 AS Exception
' TODO : 最初/最後の例外を格納しておくなり、全部リストに追加するなり
' ココがぼのぼのさんの問題視している部分?
End Try
End Try
Next i
End Sub

#End Region


うーん、さらに危険な香りがします。
余計なことをしようとすればするほど、参照カウントが増える機会が多くなります。

引数が「参照渡し」のものすごい数のオーバーロードを作ると少しは安心できるかもしれません。

# スッキリしたという部分を強調

[ メッセージ編集済み 編集者: じゃんぬねっと 編集日時 2006-10-12 11:28 ]

スキルアップ/キャリアアップ(JOB@IT)