トップ回答者
その造りはデザインパターン的にありえない?

質問
-
.net初心者のなつです。
先日職場でソースコードレビューがあったのですが、
そこで私のソースコードを見て、
「その造りはデザインパターン的にありえない」とダメ出しを受けました。
本業務は時間的な面を考慮して修正なしで進んだのですが、、、
どうしてもその意味が知りたく本人に話を聞いたのですが
初心者のわたしは相手にされず。。。
簡単に造りを説明すると、
コントローラオブジェクトに存在する
オブジェクトAを他のオブジェクトBに渡し、オブジェクトB内でオブジェクトAのデータを編集し
オブジェクトAを他のオブジェクトCに渡し、オブジェクトC内でオブジェクトAのデータを編集するということです。
機能的には問題なのですが(↓下記サンプルソース参照)
よくオブジェクト指向プログラミングやUMLなどの書籍を読むと
目にする「デザインパターン」。
どう勉強に取り掛かっていくか悩んでいます。
職場には相談できる人がいなくて、、、
よろしくお願いいたします。
SAMPLE
’フォームのクリックイベントから起動され
’DataObjectオブジェクトがControlDataObjectAオブジェクト内でデータを書き換え
’データを外出しし、
’DataObjectオブジェクトがControlDataObjectBオブジェクト内でデータを書き換え
’データを外出する。
Private Sub ChgDataEvent_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles ChgDataEvent.Click
Dim dtObj As New DataObject
Dim ctrlA As New ControlDataObjectA
Dim ctrlB As New ControlDataObjectB
ctrlA.setA = dtObj
Console.WriteLine(dtObj.GetSet)
ctrlB.setB = dtObj
Console.WriteLine(dtObj.GetSet)
End Sub
Public Class DataObject
Private outData As String
Friend Property GetSet() As String
Get
Return Me.outData
End Get
Set(ByVal value As String)
Me.outData = value
End Set
End Property
End Class
Public Class ControlDataObjectA
Friend WriteOnly Property setA() As DataObject
Set(ByVal value As DataObject)
value.GetSet = "A"
End Set
End Property
End Class
Public Class ControlDataObjectB
Friend WriteOnly Property setB() As DataObject
Set(ByVal value As DataObject)
value.GetSet = "B"
End Set
End Property
End Class
回答
-
データオブジェクトのプロパティ設定を設定パターンごとの設定クラスに任せる
という前提で書きます。
クラスライブラリ開発のデザインガイドライン
http://msdn.microsoft.com/ja-jp/library/ms229042(VS.80).aspx
その人がなぜデザインパターンと言ったのかは不明ですが
それより作り方の問題が大きいようです。
natu.s さんからの引用 Public Class ControlDataObjectA
Friend WriteOnly Property setA() As DataObject
Set(ByVal value As DataObject)
value.GetSet = "A"
End Set
End Property
End Class1.WriteOnly プロパティは非推奨
上記リンクにも書いてありますがセキュリティの問題があります。
設定できるが参照できないとはこういうことです。
預金者:「私の口座の暗証番号を教えてください。」
銀行員:「それはできません。」
預金者:「では、暗証番号を1234に変更してください。」
銀行員:「わかりました。」
2.プロパティなのに処理をしている
プロパティはそれを実装しているクラスの属性(状態)を表すものです。
どう考えてもメソッドですね。
Code SnippetFriend Sub SetA(ByVal data As DataObject)
End Sub
3.インスタンスクラスである必要が無い
コントローラはインスタンスを作る必要はないでしょう。(書かれた内容からは)
Code SnippetPublic Class ControlDataObjectA
Public Shared Sub SetA(ByVal data As DataObject)
End Sub
End Class
ファクトリパターンであれば
Code SnippetPublic Class DataObjectFactory
Public Shared Function Create(ByVal pattern As DataPattern) As DataObject
Slelect Case pattern
Case DataPattern.A
Dim newData As New DataObject
newData.Prop1 = "A"
Return newData
End Select
End Function
End Class
みたいになるでしょうか。
DataObjectを作る際はこれを呼び出すようにします。
その人が「その造りはデザインパターン的にありえない」の一言だけを言われたのだとしたら、不親切だと思います。
natu.s さんからの引用 本業務は時間的な面を考慮して修正なしで進んだのですが
上記に挙げたこと(1と2)は品質にかかわりますので、できれば直した方がいいと思います。
すべての返信
-
データオブジェクトのプロパティ設定を設定パターンごとの設定クラスに任せる
という前提で書きます。
クラスライブラリ開発のデザインガイドライン
http://msdn.microsoft.com/ja-jp/library/ms229042(VS.80).aspx
その人がなぜデザインパターンと言ったのかは不明ですが
それより作り方の問題が大きいようです。
natu.s さんからの引用 Public Class ControlDataObjectA
Friend WriteOnly Property setA() As DataObject
Set(ByVal value As DataObject)
value.GetSet = "A"
End Set
End Property
End Class1.WriteOnly プロパティは非推奨
上記リンクにも書いてありますがセキュリティの問題があります。
設定できるが参照できないとはこういうことです。
預金者:「私の口座の暗証番号を教えてください。」
銀行員:「それはできません。」
預金者:「では、暗証番号を1234に変更してください。」
銀行員:「わかりました。」
2.プロパティなのに処理をしている
プロパティはそれを実装しているクラスの属性(状態)を表すものです。
どう考えてもメソッドですね。
Code SnippetFriend Sub SetA(ByVal data As DataObject)
End Sub
3.インスタンスクラスである必要が無い
コントローラはインスタンスを作る必要はないでしょう。(書かれた内容からは)
Code SnippetPublic Class ControlDataObjectA
Public Shared Sub SetA(ByVal data As DataObject)
End Sub
End Class
ファクトリパターンであれば
Code SnippetPublic Class DataObjectFactory
Public Shared Function Create(ByVal pattern As DataPattern) As DataObject
Slelect Case pattern
Case DataPattern.A
Dim newData As New DataObject
newData.Prop1 = "A"
Return newData
End Select
End Function
End Class
みたいになるでしょうか。
DataObjectを作る際はこれを呼び出すようにします。
その人が「その造りはデザインパターン的にありえない」の一言だけを言われたのだとしたら、不親切だと思います。
natu.s さんからの引用 本業務は時間的な面を考慮して修正なしで進んだのですが
上記に挙げたこと(1と2)は品質にかかわりますので、できれば直した方がいいと思います。
-
natu.s さんからの引用 #DataObjectを他のクラスで書き換えて、
#また他のクラスでも書き換えるというのは
#作り的に問題ないのでしょうか?
特に問題はありません。
気を付けることは、参照する側と利用される側という参照方向が一方向にすることでしょう。
やり方については、
初期化という意味での設定であれば、DataObjectのコンストラクタ オーバーロードの方がわかりやすいかもしれません。
他のクラスが書き換える場合、先に書いたようなDataObjectFactoryのような形にして
インスタンスを作成するのを含めて一元化するほうが管理しやすいかもしれません。
#場合によっては、DataObjectクラスのコンストラクタをPrivateに書き換えて直接Newできないようにしたりもします。
上記の違いは、
設定パターンがDataObjectそのものの仕様なのか、DataObjectの外の世界の仕様なのか
といったところです。
#仕様を管理しているのはだれかということです。
仕様がわからないのであれですが、いずれにしろパターンごとにクラスを作るのは例を見る限り冗長だと感じます。