none
その造りはデザインパターン的にありえない? RRS feed

  • 質問

  • .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

    2008年12月6日 19:39

回答

  • データオブジェクトのプロパティ設定を設定パターンごとの設定クラスに任せる

    という前提で書きます。

     

    クラスライブラリ開発のデザインガイドライン

    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 Class

     

    1.WriteOnly プロパティは非推奨

     

    上記リンクにも書いてありますがセキュリティの問題があります。

    設定できるが参照できないとはこういうことです。

    預金者:「私の口座の暗証番号を教えてください。」

    銀行員:「それはできません。」

    預金者:「では、暗証番号を1234に変更してください。」

    銀行員:「わかりました。」

     

    2.プロパティなのに処理をしている

     

    プロパティはそれを実装しているクラスの属性(状態)を表すものです。

    どう考えてもメソッドですね。

    Code Snippet

    Friend Sub SetA(ByVal data As DataObject)

    End Sub

     

     

     

    3.インスタンスクラスである必要が無い

     

    コントローラはインスタンスを作る必要はないでしょう。(書かれた内容からは)

     

    Code Snippet

    Public Class ControlDataObjectA

        Public Shared Sub SetA(ByVal data As DataObject)

        End Sub

    End Class

     

     

    ファクトリパターンであれば

    Code Snippet

    Public 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)は品質にかかわりますので、できれば直した方がいいと思います。

     

    2008年12月7日 1:23

すべての返信

  • データオブジェクトのプロパティ設定を設定パターンごとの設定クラスに任せる

    という前提で書きます。

     

    クラスライブラリ開発のデザインガイドライン

    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 Class

     

    1.WriteOnly プロパティは非推奨

     

    上記リンクにも書いてありますがセキュリティの問題があります。

    設定できるが参照できないとはこういうことです。

    預金者:「私の口座の暗証番号を教えてください。」

    銀行員:「それはできません。」

    預金者:「では、暗証番号を1234に変更してください。」

    銀行員:「わかりました。」

     

    2.プロパティなのに処理をしている

     

    プロパティはそれを実装しているクラスの属性(状態)を表すものです。

    どう考えてもメソッドですね。

    Code Snippet

    Friend Sub SetA(ByVal data As DataObject)

    End Sub

     

     

     

    3.インスタンスクラスである必要が無い

     

    コントローラはインスタンスを作る必要はないでしょう。(書かれた内容からは)

     

    Code Snippet

    Public Class ControlDataObjectA

        Public Shared Sub SetA(ByVal data As DataObject)

        End Sub

    End Class

     

     

    ファクトリパターンであれば

    Code Snippet

    Public 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)は品質にかかわりますので、できれば直した方がいいと思います。

     

    2008年12月7日 1:23
  • まどかさんありがとうございます。

     

    作り方が大問題ですね↓

     

    おっしゃっていることがとてもわかりやすかったです。

    確かにその通りですね。

    ありがとうございます。

     

    リンクに書いてあることをよく読んで、

    作り方をしっかり勉強したいと思います。

     

     

     

     

    #プロパティをメソッドに変えたとして、

    #DataObjectを他のクラスで書き換えて、

    #また他のクラスでも書き換えるというのは

    #作り的に問題ないのでしょうか?

    2008年12月14日 15:37
  •  natu.s さんからの引用

    #DataObjectを他のクラスで書き換えて、

    #また他のクラスでも書き換えるというのは

    #作り的に問題ないのでしょうか?

     

    特に問題はありません。

    気を付けることは、参照する側と利用される側という参照方向が一方向にすることでしょう。

     

    やり方については、

    初期化という意味での設定であれば、DataObjectのコンストラクタ オーバーロードの方がわかりやすいかもしれません。

    他のクラスが書き換える場合、先に書いたようなDataObjectFactoryのような形にして

    インスタンスを作成するのを含めて一元化するほうが管理しやすいかもしれません。

    #場合によっては、DataObjectクラスのコンストラクタをPrivateに書き換えて直接Newできないようにしたりもします。

     

    上記の違いは、

    設定パターンがDataObjectそのものの仕様なのか、DataObjectの外の世界の仕様なのか

    といったところです。

    #仕様を管理しているのはだれかということです。

     

    仕様がわからないのであれですが、いずれにしろパターンごとにクラスを作るのは例を見る限り冗長だと感じます。

    2008年12月14日 18:23