- PR -

クラス設計の基本的質問:プロパティ経由で受け取ったリソースの破棄

投稿者投稿内容
Dustin
会議室デビュー日: 2007/01/30
投稿数: 7
投稿日時: 2007-01-30 19:52
はじめまして、初めて質問いたします。
C#を含むオブジェクト指向言語の経験が浅いため
クラス設計の基本について質問いたします。

IDisposableを実装するリソースオブジェクト(ここではSystem.Drawing.Penとします)の参照をプロパティ経由で受け取って保持するクラスがあるものとします。
クラスのコードは下記のとおりです。
コード:
/// <summary>領域の境界線の情報を管理</summary>
public sealed
class  TBorder : IDisposable {
	private bool   _disposed;
	private bool   _visible;
	private Pen    _pen;
	
	/// <summary>コンストラクタ</summary>
	public
	TBorder() {
		_visible = true;
		_pen     = new Pen( Color.Black, 1 );
	}
	/// <summary>デストラクタ</summary>
	~TBorder() {
		_Dispose( false );
	}

	/// <summary>境界線を表示するか</summary>
	public
	bool  Visible {
		get { return  _visible; }
		set { _visible = value; }
	}
	/// <summary>境界線のペン</summary>
	public
	Pen  Pen {
		get { return  _pen; }
		set { _pen = value; }
	}

	/// <summary>リソースを破棄します。</summary>
	public
	void  Dispose() {
		_Dispose( true );
		GC.SuppressFinalize( this );
	}
	/// <summary>リソースを破棄します。</summary>
	/// <param name="disposing">明示的にDispose()を呼ばれたか</param>
	private
	void  _Dispose( bool  disposing ) {
		if( ! _disposed ) {
			if( disposing ) {
				// ペンのリソースを破棄する
				_pen.Dispose();
				_pen = null;
			}
		}
		_disposed = true;
	}
}


受け取ったペンの破棄は、TBorderクラスの責任とします。
リソースを保持するので、TBorderにもIDisposableを実装させました。

このクラスのインスタンスを作成する側のコードで、TBorder.PenプロパティにSystem.Drawing.Pensクラスの
システム定義オブジェクト(Pens.Redなど)をセットしてDispose()メソッドを呼ぶと、上記のままでは例外が発生します。
("Changes cannot be made to Brush because permissions are not valid."のメッセージが表示されました)
システム定義オブジェクトを破棄しようとしたためと思います。
コード:
	TBorder  border;
	
	border = new TBorder();
	border.Pen = new Pen( Color.Red, 3 );
	border.Dispose();
	
	border = new TBorder();
	border.Pen = Pens.Red;
	border.Dispose(); // 例外が発生


_Dispose(bool)の"_pen.Dispose()"の部分をtry{}で囲って例外を無視すれば
この問題を回避できると考えていますが、より適切な対応がありますでしょうか。
Pens・Brushesのシステム定義オブジェクトを多用するつもりですので、
前述のtry{}で例外が発生する(例外をキャッチする)可能性が高いと思います。
事前にシステム定義オブジェクトか(Dispose()を呼ぶべきかどうか)チェックする方法が
あるものでしょうか。
あるいはクラスの設計が不適切でしょうか。
Penクラスを直接プロパティで受け渡しするのがまずいなど。

ちなみに、このクラスのようにPen・Brushのインスタンスを保持するクラスが他にも
あるのですが、これらは皆IDisposableを実装するのが適切でしょうか。
じゃんぬねっと
ぬし
会議室デビュー日: 2004/12/22
投稿数: 7811
お住まい・勤務地: 愛知県名古屋市
投稿日時: 2007-01-30 20:09
Dispose... どうする? (wankuma.com)

_________________
C# と VB.NET の入門サイト
じゃんぬねっと日誌
Dustin
会議室デビュー日: 2007/01/30
投稿数: 7
投稿日時: 2007-01-30 20:30
> Dispose... どうする? (wankuma.com)
まさにこれと同じケースですね。ありがとうございます。
よねKEN
ぬし
会議室デビュー日: 2003/08/23
投稿数: 472
投稿日時: 2007-01-31 09:56
引用:

受け取ったペンの破棄は、TBorderクラスの責任とします。



渡した側の責任とするのがよいのではないでしょうか?

「TBorderクラスの責任」とすると
PenプロパティのSet時にはそれまで保持していたPenは
Disposeしないの?という疑問が残ります。

Penプロパティの入替時に古いPenをDisposeしていないので、
コンストラクタで生成している
_pen = new Pen( Color.Black, 1 );
このPenのDisposeは呼ばれない可能性があります。
渋木宏明(ひどり)
ぬし
会議室デビュー日: 2004/01/14
投稿数: 1155
お住まい・勤務地: 東京
投稿日時: 2007-01-31 11:48
引用:

よねKENさんの書き込み (2007-01-31 09:56) より:
引用:

受け取ったペンの破棄は、TBorderクラスの責任とします。



渡した側の責任とするのがよいのではないでしょうか?



例えば、プロパティ渡しじゃなくてイベントで取りに来させるのもありですね。

あと、イベントで描画そのものを委譲するとか。。。って、オーナードローぢゃん ;-p
囚人
ぬし
会議室デビュー日: 2005/08/13
投稿数: 1019
投稿日時: 2007-01-31 12:54
C++ の new と delete と同じで「生成した者が破棄の責任を持つ」でいいと思います。
ショーモナイ記事ですが
http://blogs.wankuma.com/shuujin/archive/2006/09/05/37721.aspx

_________________
囚人のジレンマな日々
Dustin
会議室デビュー日: 2007/01/30
投稿数: 7
投稿日時: 2007-01-31 14:10
皆様ありがとうございます。
引用:

Penプロパティの入替時に古いPenをDisposeしていないので、
コンストラクタで生成している
_pen = new Pen( Color.Black, 1 );
このPenのDisposeは呼ばれない可能性があります。


!! おっしゃる通りです;;
実を申しますと、これらのクラスを設計した時点では
ペン・ブラシがシステムリソースであるという認識が私の中で薄いものでして、
上記のクラスも、もともとはIDisposableを実装させておらず、
ここでご質問するために便宜上IDisposableを実装させました。
ImageやGraphicsPathオブジェクトを保持するクラスについては
当初からIDisposableを実装させ、管理するリソースに外部から直接
アクセスさせるようなことはしていないのですが。
# PointやRectangle構造体を使うような気軽さでした;;

現在、グラフを描画する簡易なクラスを作っているところでして(C#の学習目的)、
TBorderなどペン・ブラシを保持するクラスは、他のクラスからなる階層構造の中に組み込まれております。
コード:

graph.BackBrush
graph.Border.Pen
graph.Axis.Left.Ticks.Major.Pen
graph.Axis.Right.Grid.Minor.Pen


この例ではgraph、graph.Border、graph.Axis.Left.Ticks.Major、graph.Axis.Right.Grid.Minorが
それぞれペンやブラシを保持しています。実際にはもっとクラスがあります。
グラフ描画の主体は、部品に分けて可能な限りそれぞれのクラスに分散するように設計しました。
例えばgraph.Axis.Left.Ticks.Majorはグラフ左軸の目盛の描画を、
graph.Axis.Right.Gridは右軸のグリッド線の描画を担当します。
もしペン・ブラシのリソースの生成と破棄の責任をユーザのコード側に任せることになりますと
生成と破棄のコーディングが少々頻雑になると考えました。
コード:

// グラフ描画クラスのインスタンスを作成
// (これがクラス階層のルートです)
IGraphPainter graph = TGraph.CreateGraph();
// 必要なリソースを生成
Brush brushGraph = new SolidBrush(...);
Pen penBorder = new Pen(...);
Pen penAxisLMajor = new Pen(...);
Pen penAxisRMinor = new Pen(...);

// それぞれに設定
graph.BackBrush = brushGraph;
graph.Border.Pen = penBorder;
graph.Axis.Left.Ticks.Major.Pen = penAxisLMajor;
graph.Axis.Right.Grid.Minor.Pen = penAxisRMinor;

// 描画
graph.Draw();

// リソースを破棄
brushGraph.Dispose();
penBorder.Dispose();
penAxisLMajor.Dispose();
penAxisRMinor.Dispose();
graph.Dispose(); // これは主にイメージバッファの解放が目的です
// これらはusing()でもいいですが


私の認識ではこのようなコードになると考えました。
リソースの破棄が設定される側(リソースを使う側)ということにすれば
コード:

IGraphPainter graph = TGraph.CreateGraph();
// それぞれに設定
graph.BackBrush = new SolidBrush(...);
graph.Border.Pen = new Pen(...);
graph.Axis.Left.Ticks.Major.Pen = new Pen(...);
graph.Axis.Right.Grid.Minor.Pen = new Pen(...);
// 描画
graph.Draw();
// 破棄
graph.Dispose();


このようにコーディングできるのではないかと思いました。


そこで、しぶしぶながらペン・ブラシ保持クラスにIDisposableを実装してまわろうかと
思っております。
実はここでも悩みがありまして、前述のようにクラスが階層を成しているのですが
リソースを保持するクラスはほとんど最下層に所属して(所有されて)います。
ここでIDisposableを実装するとなりますと、リソースを直接保持するクラスだけでなくて
ルート(graph)までの階層上に位置するほとんどのクラスが実装する必要があります。
と申しますのは、もともとgraph自身がイメージバッファを保持する関係でIDisposableを
実装しておりまして、このgraph.Dispose()の中で、階層内で管理する全リソースを
破棄できればスマートではないかと考えたためです。
コード:

// ユーザ側のコードで破棄
graph.Dispose();
// graph.Dispose()の内部で、所有する軸グループクラスのDispose()を呼ぶ
graph.Axis.Dispose();
// graph.Axis.Dispose()の内部で、所有する軸クラスのDispose()を呼ぶ
graph.Axis.Right.Dispose();
// graph.Axis.Right.Dispose()の内部で、所有するグリッドクラスのDispose()を呼ぶ
graph.Axis.Right.Grid.Dispose();
// 以下略


そのためには ほぼ全てのクラスにIDisposableを実装させることになるのですが、
手間がかかるので現在悩んでいるところです(設計の見直しも含めて)。
ライブラリとしての構築を意識するのであれば、IDisposableを実装して
ユーザにリソース破棄のタイミングをコントロールする術を提供すべきでしょうね・・・

引用:

例えば、プロパティ渡しじゃなくてイベントで取りに来させるのもありですね。
あと、イベントで描画そのものを委譲するとか。。。って、オーナードローぢゃん ;-p


そのような方法もあるのですね。ただ、イベントハンドラを設定するなら
おっしゃるように描画自体を委譲させる方向で検討します。
一部のクラスではデリゲート(event?)を使用して描画処理を外部に委譲できるように設計しました。


引用:

C++ の new と delete と同じで「生成した者が破棄の責任を持つ」でいいと思います。


ありがとうございます。その方法が定石でしょうか。


[ メッセージ編集済み 編集者: Dustin 編集日時 2007-01-31 14:33 ]

[ メッセージ編集済み 編集者: Dustin 編集日時 2007-01-31 14:34 ]
ゆうじゅん
ぬし
会議室デビュー日: 2004/01/16
投稿数: 347
投稿日時: 2007-01-31 17:30
引用:

コード:
IGraphPainter  graph = TGraph.CreateGraph();
// それぞれに設定
graph.BackBrush                 = new SolidBrush(...);
graph.Border.Pen                = new Pen(...);
graph.Axis.Left.Ticks.Major.Pen = new Pen(...);
graph.Axis.Right.Grid.Minor.Pen = new Pen(...);
// 描画
graph.Draw();
// 破棄
graph.Dispose();





graph.Dispose()でリソースを破棄すると以下のように設定した場合に
対応できないと思います。


コード:
IGraphPainter  graph = TGraph.CreateGraph();
// それぞれに設定
Pen blackPen = new Pen(...);

graph.BackBrush                 = new SolidBrush(...);
graph.Border.Pen                = blackPen;
graph.Axis.Left.Ticks.Major.Pen = blackPen;
graph.Axis.Right.Grid.Minor.Pen = blackPen;

// 描画
graph.Draw();
// 破棄
graph.Dispose();



結局ペン・ブラシのリソースをどのくらい作成して、どのように各クラスに
設定しているのかは、ペン・ブラシのリソースを作成したユーザ側のコード
でしか、わからないので

引用:

C++ の new と delete と同じで「生成した者が破棄の責任を持つ」



になると思います。

この場合、graphクラスは描画する際に使用するPenの情報だけ保持して
Draw()で保持していうPenの情報を元にPenを作成して、Draw()終了時
に破棄すればよいのではないでしょうか



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