OO Architecture Question

Discussion in 'ASP .Net' started by Mike Hofer, Apr 6, 2007.

  1. Mike Hofer

    Mike Hofer Guest

    In a large application I'm working on (ASP.NET 1.1, VS2003), we have a
    base class that wraps stored procedures. Essentially, this base class
    (StoredProcedureBase) encapsulates the code to set up the connection,
    transaction, command and parameters required to invoke a stored
    procedure on our SQL Server database. It provides helper methods that
    simplify the process of invoking the stored procedure so that our data
    access classes can make the call like this:

    Public Function GetBuildings(ByVal siteId As Integer) As
    BuildingCollection

    Dim buildings As BuildingCollection
    Dim connection As SqlConnection

    Try
    connection = ConnectionGenerator.GetOpenConnection()
    buildings = GetBuildingsProcedure.Execute(siteId, connection)
    Finally
    Disposer.DisposeOf(connection)
    End Try

    Return buildings

    End Function

    The stored procedure classes derive from StoredProcedureBase and look
    like this:

    Public NotInheritable Class GetBuildingsProcedure
    Inherits StoredProcedureBase

    Private Sub New(ByVal siteId As Integer, ByVal connection As
    SqlConnection)
    MyBase.New("GetBuildings", connection)
    AddParameter("@SiteID", SqlDbType.Int).Value = siteId
    End Sub

    Public Shared Function Execute(ByVal siteId As Integer, ByVal
    connection As SqlConnection)
    ValidateOpenConnection(connection)
    If siteId <= 0 Then Throw New
    ArgumentOutOfRangeException("siteId")
    Dim procedure As GetBuildingsProcedure
    Dim buildings As BuildingCollection
    Dim building As Building
    Dim reader As SqlDataReader
    Try
    procedure = New GetBuildingsProcedure(siteId, connection)
    reader = procedure.ExecuteReader()
    buildings = New BuildingCollection
    Do While reader.Read()
    building = New Building()
    Store(reader("ID"), -1, building.ID)
    Store(reader("AcceptsAdmissions"), False,
    building.AcceptsAdmissions)
    Store(reader("Description"), String.Empty,
    building.Description)
    Store(reader("Name"), String.Empty, building.Name)
    buildings.Add(building)
    Loop
    Finally
    Disposer.DisposeOf(reader, procedure)
    End Try
    Return buildings
    End Function
    End Class

    What we like about this model is that it provides *one* place to
    invoke the stored procedure, and that it's got really high functional
    cohesion. What worries me, however, is that it appears to be tightly
    coupled to the Building class. However, it takes very little code to
    invoke the stored procedure, and the result set is type-safe.

    But the tight coupling is bugging me. It's this annoying little voice
    nagging at me in the back of my head. The previous versions of this
    class weren't this typesafe, but they weren't so tightly coupled
    either. But with that lack of coupling came a lot of code. Refactoring
    the code to reduce code complexity resulted in this model; it's only
    after looking at the model that we realized we were now tightly
    coupled.

    However, I'm wondering if the tight coupling isn't worth the reduction
    in code duplication. My gut tells me that it is, simply from a
    reduction in code complexity and the corresponding ease of
    maintenance.

    So here are my questions to you folks:

    1. Would you do it this way, or would you suggest something else?
    2. Do you see any problems with this model?
    3. Is the tight coupling worth the trade-off for the reduction in code
    complexity and ease of maintenance?
    4. Am I missing other concerns that should be taken into account?

    I work in a vaccuum here; I am the lone developer at my company, so I
    don't have any peer developers to bounce ideas off of. So this is a
    desperate attempt to seek input from others with experience. Any
    guidance you folks can provide would be greatly appreciated.

    Thanks in advance,
    Mike
     
    Mike Hofer, Apr 6, 2007
    #1
    1. Advertising

  2. Mike Hofer

    Peter Duniho Guest

    On Thu, 05 Apr 2007 20:33:30 -0700, Mike Hofer <>
    wrote:

    > [...]
    > So here are my questions to you folks:
    >
    > 1. Would you do it this way, or would you suggest something else?
    > 2. Do you see any problems with this model?
    > 3. Is the tight coupling worth the trade-off for the reduction in code
    > complexity and ease of maintenance?
    > 4. Am I missing other concerns that should be taken into account?


    My apologies in advance for not answering the questions in a 1-to-1
    manner. :)

    My first thought is that I think you might be worrying about the wrong
    thing. Having a class closely coupled to some specific aspect of the data
    doesn't seem like a bad idea to me, especially for a derived class. After
    all, if you had a base class Shape and a bunch of derived classes, one of
    which was Circle, I don't think you'd worry about the Circle class being
    tightly coupled to the data required to describe a Circle. If a class is
    to represent something, then at some point there's got to be some coupling.

    One thing that does seem odd to me about your design is that is sounds as
    though your classes that involve stored procedures "know" a lot about
    database connections, access, etc. I would expect to either see a
    separate class that is more specific to the database and which the stored
    procedures class uses, or to see *only* a database class that also happens
    to know about procedures.

    Of course, I am not seeing the whole class structure and I may be reading
    too much into your description of the class hierarchy. But that's my
    impression upon reading that description.

    Another thing is that your derived class seems to be "upside down". That
    is, it has a public static method that does work, and a private
    constructor called by that public static method. Clients of the class
    never actually instantiate the class (only slightly odd), nor do they ever
    get an instance of the class (very odd). I see no point in having a class
    that can be instantiated (as opposed to being purely static) if the users
    of the class never actually get an instance of it.

    I see a couple of alternate approaches that might work better than the
    current design in this respect:

    * Make the constructor public, and the Execute method non-static. The
    user of the class would create an instance of it, and then call the
    Execute method on that instance.

    * Make the class purely static. Rather than creating an instance of
    the class in the Execute method, create an instance of the base class,
    initializing it as necessary for the specific situation (as in, do what
    the private constructor of the derived class does now) and use that
    directly.

    Of course, for all I know the base class could be suitably designed as a
    purely static class as well. If that's the case, then no instantiating of
    any class needs to occur.

    As a general rule: use a static class when the class doesn't need to
    maintain any state from one function call to another; use a non-static
    class if it does. There's nothing about your derived class that appears
    to me to require maintaining any state outside the Execute method, so it
    can be static.

    Take everything in this post, as well as any other replies, with a grain
    of salt. :) Only you can say for sure what's really the best design,
    especially given the other already-implemented parts of the application.

    Pete
     
    Peter Duniho, Apr 6, 2007
    #2
    1. Advertising

  3. Mike Hofer

    Peter Duniho Guest

    On Thu, 05 Apr 2007 21:48:22 -0700, Peter Duniho
    <> wrote:

    > [...]
    > * Make the class purely static. Rather than creating an instance
    > of the class in the Execute method, create an instance of the base
    > class, initializing it as necessary for the specific situation (as in,
    > do what the private constructor of the derived class does now) and use
    > that directly.


    I should clarify: in this alternate design, there would not necessarily
    be any reason for the buildings-specific class to be derived from the base
    StoredProcedureBase class. It would be a stand-along static class, unless
    you decide that the base class can also be static, in which case it might
    still make sense to derive from that class.

    Pete
     
    Peter Duniho, Apr 6, 2007
    #3
  4. Mike Hofer

    Mike Hofer Guest

    On Apr 6, 12:48 am, "Peter Duniho" <>
    wrote:
    > On Thu, 05 Apr 2007 20:33:30 -0700, Mike Hofer <>
    > wrote:
    >
    > > [...]
    > > So here are my questions to you folks:

    >
    > > 1. Would you do it this way, or would you suggest something else?
    > > 2. Do you see any problems with this model?
    > > 3. Is the tight coupling worth the trade-off for the reduction in code
    > > complexity and ease of maintenance?
    > > 4. Am I missing other concerns that should be taken into account?

    >
    > My apologies in advance for not answering the questions in a 1-to-1
    > manner. :)


    > My first thought is that I think you might be worrying about the wrong
    > thing. Having a class closely coupled to some specific aspect of the data
    > doesn't seem like a bad idea to me, especially for a derived class. After
    > all, if you had a base class Shape and a bunch of derived classes, one of
    > which was Circle, I don't think you'd worry about the Circle class being
    > tightly coupled to the data required to describe a Circle. If a class is
    > to represent something, then at some point there's got to be some coupling.


    That hadn't occurred to me, but you're right. (Sometimes, I get so
    blinded by the details that I forget about some of the more common
    sense issues.) It makes sense that this class, which invokes a stored
    procedure to retrieve buildings, would know what a Building looks
    like. The coupling between data model classes and the stored procedure
    classes to retrieve them from the databases is likely an acceptable
    coupling (provided that it doesn't break the private data barrier).

    > One thing that does seem odd to me about your design is that is sounds as
    > though your classes that involve stored procedures "know" a lot about
    > database connections, access, etc. I would expect to either see a
    > separate class that is more specific to the database and which the stored
    > procedures class uses, or to see *only* a database class that also happens
    > to know about procedures.
    >
    > Of course, I am not seeing the whole class structure and I may be reading
    > too much into your description of the class hierarchy. But that's my
    > impression upon reading that description.


    Actually, they don't. The database connections are handled by a
    separate class, ConnectionGenerator (a factory). Further, the
    ConnectionGenerator class doesn't know the settings; those are stored
    in the application's configuration file, and passed to the
    ConnectionGeerator during startup. GetOpenConnection() is one override
    that uses the default settings to return an open database connection
    to the caller. The retrieval of a database connection always takes
    place *outside* of the stored procedure class, nominally in in a data
    access class (BuildingDAC, for instance). The stored procedure classes
    themselves always take either a SqlConnection or a SqlTransaction as
    an argument to the Execute method, which is then forwarded to the base
    class constructor so that the internal Command object can be properly
    configured.

    > Another thing is that your derived class seems to be "upside down". That
    > is, it has a public static method that does work, and a private
    > constructor called by that public static method. Clients of the class
    > never actually instantiate the class (only slightly odd), nor do they ever
    > get an instance of the class (very odd). I see no point in having a class
    > that can be instantiated (as opposed to being purely static) if the users
    > of the class never actually get an instance of it.
    >
    > I see a couple of alternate approaches that might work better than the
    > current design in this respect:
    >
    > * Make the constructor public, and the Execute method non-static. The
    > user of the class would create an instance of it, and then call the
    > Execute method on that instance.
    >
    > * Make the class purely static. Rather than creating an instance of
    > the class in the Execute method, create an instance of the base class,
    > initializing it as necessary for the specific situation (as in, do what
    > the private constructor of the derived class does now) and use that
    > directly.
    >
    > Of course, for all I know the base class could be suitably designed as a
    > purely static class as well. If that's the case, then no instantiating of
    > any class needs to occur.
    >
    > As a general rule: use a static class when the class doesn't need to
    > maintain any state from one function call to another; use a non-static
    > class if it does. There's nothing about your derived class that appears
    > to me to require maintaining any state outside the Execute method, so it
    > can be static.


    Now that's a valid concern. Here's where I was coming from.

    I noticed early on that .NET doesn't provide a standard object to
    simplify access to stored procedures. But the steps to invoke one are
    typically the same (shown here for retrieving a set of data with a
    data reader):

    1.) Create a connection
    2.) Create a command
    3.) Create a parameters
    4.) Assign the SQL statement
    5.) Set the commandtype
    6.) Execute the command and get the data reader
    7.) Do your work with the reader
    8.) Dispose of the reader
    9.) Dispose of the command
    10.) Dispose of the connection

    That's why I wrote the class to wrap this stuff. And it made a huge
    difference. I didn't have to rewrite that same, monotonous code for
    stored procedures everywhere in my code. And if a parameter changed in
    a sproc, it was a lot easier to propagate the changes. But I digress.

    The first version of my stored procedure base class didn't use the
    Private Constructor/Public Execute model; instead, it used
    instantiable stored procedure classes, with one property for every
    parameter. In that model, however, you had to write a lot of code to
    represent a stored procedure. And the code to invoke the stored
    procedure was bloated as well.

    For instance, the DAC layer function looked like this:

    Public Function GetBuildings(ByVal siteId As Integer) As
    BuildingCollection

    Dim buildings As BuildingCollection
    Dim building As Building
    Dim connection As SqlConnection
    Dim procedure As GetBuildingsProcedure

    Try
    connection = ConnectionGenerator.GetOpenConnection()
    procedure = New GetBuildingsProcedure(connection)
    procedure.SiteId = siteId
    reader = procedure.ExecuteQuery()
    buildings = New BuildingCollection
    Do While reader.Read()
    bulding = New Building
    Store(reader("AcceptsBuilding"), False,
    building.AcceptsAdmissions)
    Store(reader("Description"), False, building.Description)
    Store(reader("ID"), -1, building.ID)
    Store(reader("SiteId"), -1, building.SiteId)
    Store(reader("Name"), String.Empty, building.Name)
    buildings.Add(building)
    Loop
    Finally
    Disposer.DisposeOf(connection)
    End Try

    Return buildings

    End Function

    (This is a poor example, since this stored procedure only takes one
    parameter. We have some stored procedures that take a lot of
    parameters, especially those that update data in the database.) The
    stored procedure class would then simply pass the parameters through
    to the Parameters collection on the internal Command object in the
    base class.

    Through refactoring, I found that there were several occasions where I
    wanted to invoke the same procedure from several different DAC
    classes. (This has proven to be the case for many stored procedures in
    the system.) The steps required to invoke the procedure were the same.
    The logical place to put the code was in the StoredProcedure class
    itself.

    Now here's the interesting bit: *Not all stored procedure classes in
    this system use the newer model.* To resolve the issue, a new base
    class was written (StoredProcedureBase) that derives from the older
    stored procedure base class (the incorrectly named StoredProcedure).
    Both classes are abstract (marked MustInherit in VB). The newer stored
    procedure classes inherit from StoredProcedureBase. The older stored
    procedure classes still use the older StoredProcedure class, and must
    be instantiated.

    However, when it comes time to add new stored procedures to the
    system, it takes a hell of a lot less time to add them using the newer
    model. There's a lot less code to write, simply because I don't have
    to write a property for each parameter, nor do I have to write code to
    populate the properties from within the DAC layer. The Execute method
    validates the parameters and throws exceptions if any of the
    parameters are invalid.

    So yes, it's a class. The base class (StoredProcedure) maintains
    state, but the derived classes (StoredProcedureBase and
    GetBuildingProcedure) do not. But the class abstracts away the details
    of the setup of the call. The static (Shared) Execute method invokes
    the private constructor to ensure that the stored procedure class is
    created correctly, and then disposed of properly. That, right there,
    is a huge boon: it ensures that Dispose is called on the stored
    procedure and that the internal Commmand is properly garbage
    collected.

    It's a long, messy story. I just wanted to make sure that the class
    instances were properly created, that the parameters were set up
    right, that the Execute method returned typesafe values, and that the
    internal objects were always garbage collected.

    >
    > Take everything in this post, as well as any other replies, with a grain
    > of salt. :) Only you can say for sure what's really the best design,
    > especially given the other already-implemented parts of the application.
    >
    > Pete


    Thanks very much for your feedback! It's appreciated!

    Mike
     
    Mike Hofer, Apr 6, 2007
    #4
  5. Mike Hofer

    Jeff Jarrell Guest

    mike,

    I have actually done a LOT of stuff around this sort of model. Aside from
    having database neutral factories for building the connection, command, and
    parm objects I use wrote a CodeSmith template to generate the wrapper code.
    It is always the same structuure and it always works. If the parms or
    result set changes, I regen the wrapper. If the concept in the wrapper can
    change, I try and keep it set so I regen all of the wrappers in batch mode.

    jeff

    "Mike Hofer" <> wrote in message
    news:...
    > On Apr 6, 12:48 am, "Peter Duniho" <>
    > wrote:
    >> On Thu, 05 Apr 2007 20:33:30 -0700, Mike Hofer <>
    >> wrote:
    >>
    >> > [...]
    >> > So here are my questions to you folks:

    >>
    >> > 1. Would you do it this way, or would you suggest something else?
    >> > 2. Do you see any problems with this model?
    >> > 3. Is the tight coupling worth the trade-off for the reduction in code
    >> > complexity and ease of maintenance?
    >> > 4. Am I missing other concerns that should be taken into account?

    >>
    >> My apologies in advance for not answering the questions in a 1-to-1
    >> manner. :)

    >
    >> My first thought is that I think you might be worrying about the wrong
    >> thing. Having a class closely coupled to some specific aspect of the
    >> data
    >> doesn't seem like a bad idea to me, especially for a derived class.
    >> After
    >> all, if you had a base class Shape and a bunch of derived classes, one of
    >> which was Circle, I don't think you'd worry about the Circle class being
    >> tightly coupled to the data required to describe a Circle. If a class is
    >> to represent something, then at some point there's got to be some
    >> coupling.

    >
    > That hadn't occurred to me, but you're right. (Sometimes, I get so
    > blinded by the details that I forget about some of the more common
    > sense issues.) It makes sense that this class, which invokes a stored
    > procedure to retrieve buildings, would know what a Building looks
    > like. The coupling between data model classes and the stored procedure
    > classes to retrieve them from the databases is likely an acceptable
    > coupling (provided that it doesn't break the private data barrier).
    >
    >> One thing that does seem odd to me about your design is that is sounds as
    >> though your classes that involve stored procedures "know" a lot about
    >> database connections, access, etc. I would expect to either see a
    >> separate class that is more specific to the database and which the stored
    >> procedures class uses, or to see *only* a database class that also
    >> happens
    >> to know about procedures.
    >>
    >> Of course, I am not seeing the whole class structure and I may be reading
    >> too much into your description of the class hierarchy. But that's my
    >> impression upon reading that description.

    >
    > Actually, they don't. The database connections are handled by a
    > separate class, ConnectionGenerator (a factory). Further, the
    > ConnectionGenerator class doesn't know the settings; those are stored
    > in the application's configuration file, and passed to the
    > ConnectionGeerator during startup. GetOpenConnection() is one override
    > that uses the default settings to return an open database connection
    > to the caller. The retrieval of a database connection always takes
    > place *outside* of the stored procedure class, nominally in in a data
    > access class (BuildingDAC, for instance). The stored procedure classes
    > themselves always take either a SqlConnection or a SqlTransaction as
    > an argument to the Execute method, which is then forwarded to the base
    > class constructor so that the internal Command object can be properly
    > configured.
    >
    >> Another thing is that your derived class seems to be "upside down". That
    >> is, it has a public static method that does work, and a private
    >> constructor called by that public static method. Clients of the class
    >> never actually instantiate the class (only slightly odd), nor do they
    >> ever
    >> get an instance of the class (very odd). I see no point in having a
    >> class
    >> that can be instantiated (as opposed to being purely static) if the users
    >> of the class never actually get an instance of it.
    >>
    >> I see a couple of alternate approaches that might work better than the
    >> current design in this respect:
    >>
    >> * Make the constructor public, and the Execute method non-static.
    >> The
    >> user of the class would create an instance of it, and then call the
    >> Execute method on that instance.
    >>
    >> * Make the class purely static. Rather than creating an instance of
    >> the class in the Execute method, create an instance of the base class,
    >> initializing it as necessary for the specific situation (as in, do what
    >> the private constructor of the derived class does now) and use that
    >> directly.
    >>
    >> Of course, for all I know the base class could be suitably designed as a
    >> purely static class as well. If that's the case, then no instantiating
    >> of
    >> any class needs to occur.
    >>
    >> As a general rule: use a static class when the class doesn't need to
    >> maintain any state from one function call to another; use a non-static
    >> class if it does. There's nothing about your derived class that appears
    >> to me to require maintaining any state outside the Execute method, so it
    >> can be static.

    >
    > Now that's a valid concern. Here's where I was coming from.
    >
    > I noticed early on that .NET doesn't provide a standard object to
    > simplify access to stored procedures. But the steps to invoke one are
    > typically the same (shown here for retrieving a set of data with a
    > data reader):
    >
    > 1.) Create a connection
    > 2.) Create a command
    > 3.) Create a parameters
    > 4.) Assign the SQL statement
    > 5.) Set the commandtype
    > 6.) Execute the command and get the data reader
    > 7.) Do your work with the reader
    > 8.) Dispose of the reader
    > 9.) Dispose of the command
    > 10.) Dispose of the connection
    >
    > That's why I wrote the class to wrap this stuff. And it made a huge
    > difference. I didn't have to rewrite that same, monotonous code for
    > stored procedures everywhere in my code. And if a parameter changed in
    > a sproc, it was a lot easier to propagate the changes. But I digress.
    >
    > The first version of my stored procedure base class didn't use the
    > Private Constructor/Public Execute model; instead, it used
    > instantiable stored procedure classes, with one property for every
    > parameter. In that model, however, you had to write a lot of code to
    > represent a stored procedure. And the code to invoke the stored
    > procedure was bloated as well.
    >
    > For instance, the DAC layer function looked like this:
    >
    > Public Function GetBuildings(ByVal siteId As Integer) As
    > BuildingCollection
    >
    > Dim buildings As BuildingCollection
    > Dim building As Building
    > Dim connection As SqlConnection
    > Dim procedure As GetBuildingsProcedure
    >
    > Try
    > connection = ConnectionGenerator.GetOpenConnection()
    > procedure = New GetBuildingsProcedure(connection)
    > procedure.SiteId = siteId
    > reader = procedure.ExecuteQuery()
    > buildings = New BuildingCollection
    > Do While reader.Read()
    > bulding = New Building
    > Store(reader("AcceptsBuilding"), False,
    > building.AcceptsAdmissions)
    > Store(reader("Description"), False, building.Description)
    > Store(reader("ID"), -1, building.ID)
    > Store(reader("SiteId"), -1, building.SiteId)
    > Store(reader("Name"), String.Empty, building.Name)
    > buildings.Add(building)
    > Loop
    > Finally
    > Disposer.DisposeOf(connection)
    > End Try
    >
    > Return buildings
    >
    > End Function
    >
    > (This is a poor example, since this stored procedure only takes one
    > parameter. We have some stored procedures that take a lot of
    > parameters, especially those that update data in the database.) The
    > stored procedure class would then simply pass the parameters through
    > to the Parameters collection on the internal Command object in the
    > base class.
    >
    > Through refactoring, I found that there were several occasions where I
    > wanted to invoke the same procedure from several different DAC
    > classes. (This has proven to be the case for many stored procedures in
    > the system.) The steps required to invoke the procedure were the same.
    > The logical place to put the code was in the StoredProcedure class
    > itself.
    >
    > Now here's the interesting bit: *Not all stored procedure classes in
    > this system use the newer model.* To resolve the issue, a new base
    > class was written (StoredProcedureBase) that derives from the older
    > stored procedure base class (the incorrectly named StoredProcedure).
    > Both classes are abstract (marked MustInherit in VB). The newer stored
    > procedure classes inherit from StoredProcedureBase. The older stored
    > procedure classes still use the older StoredProcedure class, and must
    > be instantiated.
    >
    > However, when it comes time to add new stored procedures to the
    > system, it takes a hell of a lot less time to add them using the newer
    > model. There's a lot less code to write, simply because I don't have
    > to write a property for each parameter, nor do I have to write code to
    > populate the properties from within the DAC layer. The Execute method
    > validates the parameters and throws exceptions if any of the
    > parameters are invalid.
    >
    > So yes, it's a class. The base class (StoredProcedure) maintains
    > state, but the derived classes (StoredProcedureBase and
    > GetBuildingProcedure) do not. But the class abstracts away the details
    > of the setup of the call. The static (Shared) Execute method invokes
    > the private constructor to ensure that the stored procedure class is
    > created correctly, and then disposed of properly. That, right there,
    > is a huge boon: it ensures that Dispose is called on the stored
    > procedure and that the internal Commmand is properly garbage
    > collected.
    >
    > It's a long, messy story. I just wanted to make sure that the class
    > instances were properly created, that the parameters were set up
    > right, that the Execute method returned typesafe values, and that the
    > internal objects were always garbage collected.
    >
    >>
    >> Take everything in this post, as well as any other replies, with a grain
    >> of salt. :) Only you can say for sure what's really the best design,
    >> especially given the other already-implemented parts of the application.
    >>
    >> Pete

    >
    > Thanks very much for your feedback! It's appreciated!
    >
    > Mike
    >
     
    Jeff Jarrell, Apr 6, 2007
    #5
  6. Mike Hofer

    John Wright Guest

    To solve the issue here I wrote a generic DAL class using the
    System.Data.Common namespace and the factory pattern. The class uses a
    parameter collection and will call any stored procedure on the database
    regardless of the backend (SQL Server, Oracle, Text Files, Access, ODBC
    database...). We hit a few snags in the Oracle arena trying to get a cursor
    object back (similiar to calling a stored proc in SQL Server that returns a
    data table), but we found a way around this that works quite well. Dropping
    the DAL into any program allows the developer to wrap it with their own
    functions to type variables but leaves it open enough to switch databases on
    the fly (I have a program that has to access both SQL Server and Oracle. It
    connects to both on the fly without any problem.) So far it was worked
    beautifully on both WinForms and ASP.NET.

    Maybe the Common namespace is something to explore.

    John
    "Jeff Jarrell" <> wrote in message
    news:...
    > mike,
    >
    > I have actually done a LOT of stuff around this sort of model. Aside from
    > having database neutral factories for building the connection, command,
    > and parm objects I use wrote a CodeSmith template to generate the wrapper
    > code. It is always the same structuure and it always works. If the parms
    > or result set changes, I regen the wrapper. If the concept in the wrapper
    > can change, I try and keep it set so I regen all of the wrappers in batch
    > mode.
    >
    > jeff
    >
    > "Mike Hofer" <> wrote in message
    > news:...
    >> On Apr 6, 12:48 am, "Peter Duniho" <>
    >> wrote:
    >>> On Thu, 05 Apr 2007 20:33:30 -0700, Mike Hofer <>
    >>> wrote:
    >>>
    >>> > [...]
    >>> > So here are my questions to you folks:
    >>>
    >>> > 1. Would you do it this way, or would you suggest something else?
    >>> > 2. Do you see any problems with this model?
    >>> > 3. Is the tight coupling worth the trade-off for the reduction in code
    >>> > complexity and ease of maintenance?
    >>> > 4. Am I missing other concerns that should be taken into account?
    >>>
    >>> My apologies in advance for not answering the questions in a 1-to-1
    >>> manner. :)

    >>
    >>> My first thought is that I think you might be worrying about the wrong
    >>> thing. Having a class closely coupled to some specific aspect of the
    >>> data
    >>> doesn't seem like a bad idea to me, especially for a derived class.
    >>> After
    >>> all, if you had a base class Shape and a bunch of derived classes, one
    >>> of
    >>> which was Circle, I don't think you'd worry about the Circle class being
    >>> tightly coupled to the data required to describe a Circle. If a class
    >>> is
    >>> to represent something, then at some point there's got to be some
    >>> coupling.

    >>
    >> That hadn't occurred to me, but you're right. (Sometimes, I get so
    >> blinded by the details that I forget about some of the more common
    >> sense issues.) It makes sense that this class, which invokes a stored
    >> procedure to retrieve buildings, would know what a Building looks
    >> like. The coupling between data model classes and the stored procedure
    >> classes to retrieve them from the databases is likely an acceptable
    >> coupling (provided that it doesn't break the private data barrier).
    >>
    >>> One thing that does seem odd to me about your design is that is sounds
    >>> as
    >>> though your classes that involve stored procedures "know" a lot about
    >>> database connections, access, etc. I would expect to either see a
    >>> separate class that is more specific to the database and which the
    >>> stored
    >>> procedures class uses, or to see *only* a database class that also
    >>> happens
    >>> to know about procedures.
    >>>
    >>> Of course, I am not seeing the whole class structure and I may be
    >>> reading
    >>> too much into your description of the class hierarchy. But that's my
    >>> impression upon reading that description.

    >>
    >> Actually, they don't. The database connections are handled by a
    >> separate class, ConnectionGenerator (a factory). Further, the
    >> ConnectionGenerator class doesn't know the settings; those are stored
    >> in the application's configuration file, and passed to the
    >> ConnectionGeerator during startup. GetOpenConnection() is one override
    >> that uses the default settings to return an open database connection
    >> to the caller. The retrieval of a database connection always takes
    >> place *outside* of the stored procedure class, nominally in in a data
    >> access class (BuildingDAC, for instance). The stored procedure classes
    >> themselves always take either a SqlConnection or a SqlTransaction as
    >> an argument to the Execute method, which is then forwarded to the base
    >> class constructor so that the internal Command object can be properly
    >> configured.
    >>
    >>> Another thing is that your derived class seems to be "upside down".
    >>> That
    >>> is, it has a public static method that does work, and a private
    >>> constructor called by that public static method. Clients of the class
    >>> never actually instantiate the class (only slightly odd), nor do they
    >>> ever
    >>> get an instance of the class (very odd). I see no point in having a
    >>> class
    >>> that can be instantiated (as opposed to being purely static) if the
    >>> users
    >>> of the class never actually get an instance of it.
    >>>
    >>> I see a couple of alternate approaches that might work better than the
    >>> current design in this respect:
    >>>
    >>> * Make the constructor public, and the Execute method non-static.
    >>> The
    >>> user of the class would create an instance of it, and then call the
    >>> Execute method on that instance.
    >>>
    >>> * Make the class purely static. Rather than creating an instance
    >>> of
    >>> the class in the Execute method, create an instance of the base class,
    >>> initializing it as necessary for the specific situation (as in, do what
    >>> the private constructor of the derived class does now) and use that
    >>> directly.
    >>>
    >>> Of course, for all I know the base class could be suitably designed as a
    >>> purely static class as well. If that's the case, then no instantiating
    >>> of
    >>> any class needs to occur.
    >>>
    >>> As a general rule: use a static class when the class doesn't need to
    >>> maintain any state from one function call to another; use a non-static
    >>> class if it does. There's nothing about your derived class that appears
    >>> to me to require maintaining any state outside the Execute method, so it
    >>> can be static.

    >>
    >> Now that's a valid concern. Here's where I was coming from.
    >>
    >> I noticed early on that .NET doesn't provide a standard object to
    >> simplify access to stored procedures. But the steps to invoke one are
    >> typically the same (shown here for retrieving a set of data with a
    >> data reader):
    >>
    >> 1.) Create a connection
    >> 2.) Create a command
    >> 3.) Create a parameters
    >> 4.) Assign the SQL statement
    >> 5.) Set the commandtype
    >> 6.) Execute the command and get the data reader
    >> 7.) Do your work with the reader
    >> 8.) Dispose of the reader
    >> 9.) Dispose of the command
    >> 10.) Dispose of the connection
    >>
    >> That's why I wrote the class to wrap this stuff. And it made a huge
    >> difference. I didn't have to rewrite that same, monotonous code for
    >> stored procedures everywhere in my code. And if a parameter changed in
    >> a sproc, it was a lot easier to propagate the changes. But I digress.
    >>
    >> The first version of my stored procedure base class didn't use the
    >> Private Constructor/Public Execute model; instead, it used
    >> instantiable stored procedure classes, with one property for every
    >> parameter. In that model, however, you had to write a lot of code to
    >> represent a stored procedure. And the code to invoke the stored
    >> procedure was bloated as well.
    >>
    >> For instance, the DAC layer function looked like this:
    >>
    >> Public Function GetBuildings(ByVal siteId As Integer) As
    >> BuildingCollection
    >>
    >> Dim buildings As BuildingCollection
    >> Dim building As Building
    >> Dim connection As SqlConnection
    >> Dim procedure As GetBuildingsProcedure
    >>
    >> Try
    >> connection = ConnectionGenerator.GetOpenConnection()
    >> procedure = New GetBuildingsProcedure(connection)
    >> procedure.SiteId = siteId
    >> reader = procedure.ExecuteQuery()
    >> buildings = New BuildingCollection
    >> Do While reader.Read()
    >> bulding = New Building
    >> Store(reader("AcceptsBuilding"), False,
    >> building.AcceptsAdmissions)
    >> Store(reader("Description"), False, building.Description)
    >> Store(reader("ID"), -1, building.ID)
    >> Store(reader("SiteId"), -1, building.SiteId)
    >> Store(reader("Name"), String.Empty, building.Name)
    >> buildings.Add(building)
    >> Loop
    >> Finally
    >> Disposer.DisposeOf(connection)
    >> End Try
    >>
    >> Return buildings
    >>
    >> End Function
    >>
    >> (This is a poor example, since this stored procedure only takes one
    >> parameter. We have some stored procedures that take a lot of
    >> parameters, especially those that update data in the database.) The
    >> stored procedure class would then simply pass the parameters through
    >> to the Parameters collection on the internal Command object in the
    >> base class.
    >>
    >> Through refactoring, I found that there were several occasions where I
    >> wanted to invoke the same procedure from several different DAC
    >> classes. (This has proven to be the case for many stored procedures in
    >> the system.) The steps required to invoke the procedure were the same.
    >> The logical place to put the code was in the StoredProcedure class
    >> itself.
    >>
    >> Now here's the interesting bit: *Not all stored procedure classes in
    >> this system use the newer model.* To resolve the issue, a new base
    >> class was written (StoredProcedureBase) that derives from the older
    >> stored procedure base class (the incorrectly named StoredProcedure).
    >> Both classes are abstract (marked MustInherit in VB). The newer stored
    >> procedure classes inherit from StoredProcedureBase. The older stored
    >> procedure classes still use the older StoredProcedure class, and must
    >> be instantiated.
    >>
    >> However, when it comes time to add new stored procedures to the
    >> system, it takes a hell of a lot less time to add them using the newer
    >> model. There's a lot less code to write, simply because I don't have
    >> to write a property for each parameter, nor do I have to write code to
    >> populate the properties from within the DAC layer. The Execute method
    >> validates the parameters and throws exceptions if any of the
    >> parameters are invalid.
    >>
    >> So yes, it's a class. The base class (StoredProcedure) maintains
    >> state, but the derived classes (StoredProcedureBase and
    >> GetBuildingProcedure) do not. But the class abstracts away the details
    >> of the setup of the call. The static (Shared) Execute method invokes
    >> the private constructor to ensure that the stored procedure class is
    >> created correctly, and then disposed of properly. That, right there,
    >> is a huge boon: it ensures that Dispose is called on the stored
    >> procedure and that the internal Commmand is properly garbage
    >> collected.
    >>
    >> It's a long, messy story. I just wanted to make sure that the class
    >> instances were properly created, that the parameters were set up
    >> right, that the Execute method returned typesafe values, and that the
    >> internal objects were always garbage collected.
    >>
    >>>
    >>> Take everything in this post, as well as any other replies, with a grain
    >>> of salt. :) Only you can say for sure what's really the best design,
    >>> especially given the other already-implemented parts of the application.
    >>>
    >>> Pete

    >>
    >> Thanks very much for your feedback! It's appreciated!
    >>
    >> Mike
    >>

    >
    >
     
    John Wright, Apr 6, 2007
    #6
  7. Mike Hofer

    sloan Guest

    Here are my thoughts:

    If you're a lone developer, then don't reinvent the wheel.

    Go get either the "Data Access Application Block 2.0" (sql server specific)
    or the
    EnterpriseLibrary.Data ( factory setup with support for Sql Server, Oracle )


    If you go here:
    http://sholliday.spaces.live.com/blog/
    and find this entry:
    5/24/2006
    Custom Objects/Collections and Tiered Development

    You can see how I'm taking IDataReaders, and making them into objects, even
    hierarchy ones ( Customers with Orders ).

    I also describe the approach here:
    http://www.codeproject.com/useritem...rumid=322471&exp=0&select=1954453#xx1954453xx

    If you need to populate a Circle (and its a Shape), you still gotta write a
    query/stored procedure that gets back Circle data.
    Either that, or you'll have 1,000 if statements in your idatareader to
    object code.
    (if type in db is a circle, then set these properties )
    (if type in db is a rectangle, hten set these properties)

    I wouldn't do that. I'd have a "deserializer" (as in my blog code) specific
    to each object.

    Like I said, I wouldn't reinvent the wheel. The EnterpriseLibrary.Data is
    something you can trust, and trust people alot smarter than you and me
    combined help put together.

    I like my approach (which has been fine tuned over hte last 3 years, and
    with help from a guy who helped me understand how to write code that didn't
    care about whether your backend db was sql server, oracle, access or any
    other datastore).



    "Mike Hofer" <> wrote in message
    news:...
    > In a large application I'm working on (ASP.NET 1.1, VS2003), we have a
    > base class that wraps stored procedures. Essentially, this base class
    > (StoredProcedureBase) encapsulates the code to set up the connection,
    > transaction, command and parameters required to invoke a stored
    > procedure on our SQL Server database. It provides helper methods that
    > simplify the process of invoking the stored procedure so that our data
    > access classes can make the call like this:
    >
    > Public Function GetBuildings(ByVal siteId As Integer) As
    > BuildingCollection
    >
    > Dim buildings As BuildingCollection
    > Dim connection As SqlConnection
    >
    > Try
    > connection = ConnectionGenerator.GetOpenConnection()
    > buildings = GetBuildingsProcedure.Execute(siteId, connection)
    > Finally
    > Disposer.DisposeOf(connection)
    > End Try
    >
    > Return buildings
    >
    > End Function
    >
    > The stored procedure classes derive from StoredProcedureBase and look
    > like this:
    >
    > Public NotInheritable Class GetBuildingsProcedure
    > Inherits StoredProcedureBase
    >
    > Private Sub New(ByVal siteId As Integer, ByVal connection As
    > SqlConnection)
    > MyBase.New("GetBuildings", connection)
    > AddParameter("@SiteID", SqlDbType.Int).Value = siteId
    > End Sub
    >
    > Public Shared Function Execute(ByVal siteId As Integer, ByVal
    > connection As SqlConnection)
    > ValidateOpenConnection(connection)
    > If siteId <= 0 Then Throw New
    > ArgumentOutOfRangeException("siteId")
    > Dim procedure As GetBuildingsProcedure
    > Dim buildings As BuildingCollection
    > Dim building As Building
    > Dim reader As SqlDataReader
    > Try
    > procedure = New GetBuildingsProcedure(siteId, connection)
    > reader = procedure.ExecuteReader()
    > buildings = New BuildingCollection
    > Do While reader.Read()
    > building = New Building()
    > Store(reader("ID"), -1, building.ID)
    > Store(reader("AcceptsAdmissions"), False,
    > building.AcceptsAdmissions)
    > Store(reader("Description"), String.Empty,
    > building.Description)
    > Store(reader("Name"), String.Empty, building.Name)
    > buildings.Add(building)
    > Loop
    > Finally
    > Disposer.DisposeOf(reader, procedure)
    > End Try
    > Return buildings
    > End Function
    > End Class
    >
    > What we like about this model is that it provides *one* place to
    > invoke the stored procedure, and that it's got really high functional
    > cohesion. What worries me, however, is that it appears to be tightly
    > coupled to the Building class. However, it takes very little code to
    > invoke the stored procedure, and the result set is type-safe.
    >
    > But the tight coupling is bugging me. It's this annoying little voice
    > nagging at me in the back of my head. The previous versions of this
    > class weren't this typesafe, but they weren't so tightly coupled
    > either. But with that lack of coupling came a lot of code. Refactoring
    > the code to reduce code complexity resulted in this model; it's only
    > after looking at the model that we realized we were now tightly
    > coupled.
    >
    > However, I'm wondering if the tight coupling isn't worth the reduction
    > in code duplication. My gut tells me that it is, simply from a
    > reduction in code complexity and the corresponding ease of
    > maintenance.
    >
    > So here are my questions to you folks:
    >
    > 1. Would you do it this way, or would you suggest something else?
    > 2. Do you see any problems with this model?
    > 3. Is the tight coupling worth the trade-off for the reduction in code
    > complexity and ease of maintenance?
    > 4. Am I missing other concerns that should be taken into account?
    >
    > I work in a vaccuum here; I am the lone developer at my company, so I
    > don't have any peer developers to bounce ideas off of. So this is a
    > desperate attempt to seek input from others with experience. Any
    > guidance you folks can provide would be greatly appreciated.
    >
    > Thanks in advance,
    > Mike
    >
     
    sloan, Apr 6, 2007
    #7
  8. Mike Hofer

    Peter Duniho Guest

    On Fri, 06 Apr 2007 04:34:01 -0700, Mike Hofer <>
    wrote:

    > [...]
    > So yes, it's a class. The base class (StoredProcedure) maintains
    > state, but the derived classes (StoredProcedureBase and
    > GetBuildingProcedure) do not. But the class abstracts away the details
    > of the setup of the call. The static (Shared) Execute method invokes
    > the private constructor to ensure that the stored procedure class is
    > created correctly, and then disposed of properly. That, right there,
    > is a huge boon: it ensures that Dispose is called on the stored
    > procedure and that the internal Commmand is properly garbage
    > collected.


    Well, that brings be me back to my previous suggestion:

    It sounds as though your object-specific classes aren't actually instances
    of the stored procedure base class. They are users of that class. No
    state is stored in the derived class and the callers never see the
    instance you do make. So it seems to me it makes more sense to not bother
    deriving the object-specific classes from the stored procedure class, and
    instead just have them be static classes that user the base stored
    procedure class in their static method.

    If it ever becomes the case that the object-specific classes *do* need to
    store state, then it's practically certain that will mean that users of
    the class will at that point need to instantiate the class and keep the
    instance reference. So it's not as though your current design won't
    require you to revisit each and every use of the class anyway if you make
    that change.

    There's nothing technically wrong with the way you're doing it now, but it
    does seem to be a departure from usual OOP design, and that sort of thing
    can make code harder to maintain later on (as someone, perhaps even you,
    looks at it and scratches their head wondering "why'd it get written like
    this?" :) .

    Anyway, that's about all I have in the way of potentially useful input. I
    don't feel strongly that the current design is wrong...it's just a
    suggestion about how it might be improved to be a little confusing. :)

    Pete
     
    Peter Duniho, Apr 6, 2007
    #8
  9. Mike Hofer

    Mike Hofer Guest

    On Apr 6, 12:01 pm, "sloan" <> wrote:
    > Here are my thoughts:
    >
    > If you're a lone developer, then don't reinvent the wheel.
    >
    > Go get either the "Data Access Application Block 2.0" (sql server specific)
    > or the
    > EnterpriseLibrary.Data ( factory setup with support for Sql Server, Oracle )
    >
    > If you go here:http://sholliday.spaces.live.com/blog/
    > and find this entry:
    > 5/24/2006
    > Custom Objects/Collections and Tiered Development
    >
    > You can see how I'm taking IDataReaders, and making them into objects, even
    > hierarchy ones ( Customers with Orders ).
    >
    > I also describe the approach here:http://www.codeproject.com/useritems/BusinessObjectHelper.asp?df=100&...
    >
    > If you need to populate a Circle (and its a Shape), you still gotta write a
    > query/stored procedure that gets back Circle data.
    > Either that, or you'll have 1,000 if statements in your idatareader to
    > object code.
    > (if type in db is a circle, then set these properties )
    > (if type in db is a rectangle, hten set these properties)
    >
    > I wouldn't do that. I'd have a "deserializer" (as in my blog code) specific
    > to each object.
    >
    > Like I said, I wouldn't reinvent the wheel. The EnterpriseLibrary.Data is
    > something you can trust, and trust people alot smarter than you and me
    > combined help put together.
    >
    > I like my approach (which has been fine tuned over hte last 3 years, and
    > with help from a guy who helped me understand how to write code that didn't
    > care about whether your backend db was sql server, oracle, access or any
    > other datastore).
    >
    > "Mike Hofer" <> wrote in message
    >
    > news:...
    >
    >
    >
    > > In a large application I'm working on (ASP.NET 1.1, VS2003), we have a
    > > base class that wraps stored procedures. Essentially, this base class
    > > (StoredProcedureBase) encapsulates the code to set up the connection,
    > > transaction, command and parameters required to invoke a stored
    > > procedure on our SQL Server database. It provides helper methods that
    > > simplify the process of invoking the stored procedure so that our data
    > > access classes can make the call like this:

    >
    > > Public Function GetBuildings(ByVal siteId As Integer) As
    > > BuildingCollection

    >
    > > Dim buildings As BuildingCollection
    > > Dim connection As SqlConnection

    >
    > > Try
    > > connection = ConnectionGenerator.GetOpenConnection()
    > > buildings = GetBuildingsProcedure.Execute(siteId, connection)
    > > Finally
    > > Disposer.DisposeOf(connection)
    > > End Try

    >
    > > Return buildings

    >
    > > End Function

    >
    > > The stored procedure classes derive from StoredProcedureBase and look
    > > like this:

    >
    > > Public NotInheritable Class GetBuildingsProcedure
    > > Inherits StoredProcedureBase

    >
    > > Private Sub New(ByVal siteId As Integer, ByVal connection As
    > > SqlConnection)
    > > MyBase.New("GetBuildings", connection)
    > > AddParameter("@SiteID", SqlDbType.Int).Value = siteId
    > > End Sub

    >
    > > Public Shared Function Execute(ByVal siteId As Integer, ByVal
    > > connection As SqlConnection)
    > > ValidateOpenConnection(connection)
    > > If siteId <= 0 Then Throw New
    > > ArgumentOutOfRangeException("siteId")
    > > Dim procedure As GetBuildingsProcedure
    > > Dim buildings As BuildingCollection
    > > Dim building As Building
    > > Dim reader As SqlDataReader
    > > Try
    > > procedure = New GetBuildingsProcedure(siteId, connection)
    > > reader = procedure.ExecuteReader()
    > > buildings = New BuildingCollection
    > > Do While reader.Read()
    > > building = New Building()
    > > Store(reader("ID"), -1, building.ID)
    > > Store(reader("AcceptsAdmissions"), False,
    > > building.AcceptsAdmissions)
    > > Store(reader("Description"), String.Empty,
    > > building.Description)
    > > Store(reader("Name"), String.Empty, building.Name)
    > > buildings.Add(building)
    > > Loop
    > > Finally
    > > Disposer.DisposeOf(reader, procedure)
    > > End Try
    > > Return buildings
    > > End Function
    > > End Class

    >
    > > What we like about this model is that it provides *one* place to
    > > invoke the stored procedure, and that it's got really high functional
    > > cohesion. What worries me, however, is that it appears to be tightly
    > > coupled to the Building class. However, it takes very little code to
    > > invoke the stored procedure, and the result set is type-safe.

    >
    > > But the tight coupling is bugging me. It's this annoying little voice
    > > nagging at me in the back of my head. The previous versions of this
    > > class weren't this typesafe, but they weren't so tightly coupled
    > > either. But with that lack of coupling came a lot of code. Refactoring
    > > the code to reduce code complexity resulted in this model; it's only
    > > after looking at the model that we realized we were now tightly
    > > coupled.

    >
    > > However, I'm wondering if the tight coupling isn't worth the reduction
    > > in code duplication. My gut tells me that it is, simply from a
    > > reduction in code complexity and the corresponding ease of
    > > maintenance.

    >
    > > So here are my questions to you folks:

    >
    > > 1. Would you do it this way, or would you suggest something else?
    > > 2. Do you see any problems with this model?
    > > 3. Is the tight coupling worth the trade-off for the reduction in code
    > > complexity and ease of maintenance?
    > > 4. Am I missing other concerns that should be taken into account?

    >
    > > I work in a vaccuum here; I am the lone developer at my company, so I
    > > don't have any peer developers to bounce ideas off of. So this is a
    > > desperate attempt to seek input from others with experience. Any
    > > guidance you folks can provide would be greatly appreciated.

    >
    > > Thanks in advance,
    > > Mike- Hide quoted text -

    >
    > - Show quoted text -


    Hiya sloan,

    Thanks for the pointer. I actually looked at both of these solutions.
    Unfortunately, the vast majority of the code in question was generated
    by another tool. There are 600 or so classes generated to do *exactly*
    what you're describing. That is, get the data from the stored
    procedure, shuffle it into a typesafe object (or typesafe collection),
    etc. Essentially, every time the database schema changed, the tool
    would regenerate the stored procecure classes, the data model classes,
    and the data access classes so that the code stayed in synch with the
    database. (It used base classes for the generated code, and provided
    stub derived classes for you to provide extensions.)

    Now, I don't mind the typesafe nature of the classes. Far from it. I
    *like* the typesafety. I also *like* the fact that I don't have
    DataSets all over my application. (Let's not get into that; suffice it
    to say that I think a DataSet is overkill when you know exactly what
    you want out of the database.)

    Anyway, the application is now two years old, and the amount of
    generated code makes a change of architecture on that scale
    impractical. Inserting a derived class into the inheritance chain
    wasn't, and it solved a good number of problems. The code is fast,
    typesafe, and very efficient at garbage collection.

    Now, future projects, built from scratch...that's a different story.
    But those applications will likely be done in .NET 2.0 anyway.

    Mike
     
    Mike Hofer, Apr 6, 2007
    #9
  10. Mike Hofer

    Mike Hofer Guest

    On Apr 6, 12:34 pm, "Peter Duniho" <>
    wrote:
    > On Fri, 06 Apr 2007 04:34:01 -0700, Mike Hofer <>
    > wrote:
    >
    > > [...]
    > > So yes, it's a class. The base class (StoredProcedure) maintains
    > > state, but the derived classes (StoredProcedureBase and
    > > GetBuildingProcedure) do not. But the class abstracts away the details
    > > of the setup of the call. The static (Shared) Execute method invokes
    > > the private constructor to ensure that the stored procedure class is
    > > created correctly, and then disposed of properly. That, right there,
    > > is a huge boon: it ensures that Dispose is called on the stored
    > > procedure and that the internal Commmand is properly garbage
    > > collected.

    >
    > Well, that brings be me back to my previous suggestion:
    >
    > It sounds as though your object-specific classes aren't actually instances
    > of the stored procedure base class. They are users of that class. No
    > state is stored in the derived class and the callers never see the
    > instance you do make. So it seems to me it makes more sense to not bother
    > deriving the object-specific classes from the stored procedure class, and
    > instead just have them be static classes that user the base stored
    > procedure class in their static method.
    >
    > If it ever becomes the case that the object-specific classes *do* need to
    > store state, then it's practically certain that will mean that users of
    > the class will at that point need to instantiate the class and keep the
    > instance reference. So it's not as though your current design won't
    > require you to revisit each and every use of the class anyway if you make
    > that change.
    >
    > There's nothing technically wrong with the way you're doing it now, but it
    > does seem to be a departure from usual OOP design, and that sort of thing
    > can make code harder to maintain later on (as someone, perhaps even you,
    > looks at it and scratches their head wondering "why'd it get written like
    > this?" :) .
    >
    > Anyway, that's about all I have in the way of potentially useful input. I
    > don't feel strongly that the current design is wrong...it's just a
    > suggestion about how it might be improved to be a little confusing. :)
    >
    > Pete


    I think you may be missing some points about the way the objects are
    designed. It's nothing major, just probably not obvious.

    The interface on the StoredProcedureBase class looks like this:

    Public MustInherit Class StoredProcedureBase
    Inherits StoredProcedure

    Protected Sub New(ByVal procedureName As String, ByVal connection
    As SqlConnection)
    Protected Sub New(ByVal procedureName As String, ByVal transaction
    As SqlTransaction)

    Protected Shadows Function ExecuteNonQuery() As Integer
    Protected Shadows Function ExecuteReader() As SqlDataReader
    Protected Shadows Function ExecuteScalar() As Object

    End Class

    The interface on the StoredProcedure class (which was generated) looks
    like this:

    Public MustInherit Class StoredProcedure
    Protected Sub New(ByVal procedureName As String)
    Protected Sub New(ByVal procedureName As String, ByVal
    connectionString As String)
    Protected Sub New(ByVal procedureName As String, ByVal connection
    As SqlConnection)
    Protected Sub New()

    Protected Function AddParameter(ByVal name As String, ByVal [type]
    As SqlDbType) As SqlParameter
    Protected Function AddParameter(ByVal name As String, ByVal [type]
    As SqlDbType, ByVal value As Object) As SqlParameter
    Protected Function AddParameter(ByVal name As String, ByVal [type]
    As SqlDbType, ByVal size As Integer) As SqlParameter
    Protected Function AddParameter(ByVal name As String, ByVal [type]
    As SqlDbType, ByVal direction As ParameterDirection) As SqlParameter
    Protected Function AddParameter(ByVal name As String, ByVal [type]
    As SqlDbType, ByVal size As Integer, ByVal value As Object) As
    SqlParameter

    Private Shared m_defaultConnectionString As String
    Public Shared Property DefaultConnectionString() As String

    Protected Property IsDisposed() As Boolean

    Public Overridable Sub Dispose() Implements IDisposable.Dispose

    Public Function ExecuteNonQuery() As Integer
    Public Function ExecuteReader() As SqlDataReader
    Public Function ExecuteScalar() As Object

    Protected ReadOnly Property Parameters() As SqlParameterCollection

    Public Property Transaction() As SqlTransaction

    End class

    Internally, StoredProcedure maintains a reference to a SqlCommand and
    a SqlConnection. These are the pieces of state that are maintained by
    instances of derived classes. When a class is derived from
    StoredProcedureBase, it does, indeed, inherit these pieces of state
    data. The fact that the caller never directly instantiates them is
    neither here nor there; the instantiation is abstracted away in order
    to ensure that the Dispose method is invoked.

    I had a number of issues with the generated StoredProcedure class.
    First, it provided constructors that allowed you to call it without
    providing a connection or a transaction. The result was that one was
    *automatically* created for you, which was *highly* inefficient. I
    wanted to lock down the constructors.

    Second, the Dispose method in the generated StoredProcedure class was
    buggy. It involved fairly erroneous assumptions about whether or not
    it was safe to dispose of the connection. Sometimes it disposed of it
    prematurely; other times, it didn't dispose of it at all. The newer
    class resolves the issue by demanding that a connection or a
    transaction be provided. Connections are never automagically created.

    Anyway, there's not really any big hangup. The class does maintain
    state; it just maintains it for an extremely short period of time. I
    wondered periodically if the whole thing wasn't overengineered. But
    every time I think about dispensing with the class, I keep thinking
    about how much duplicate code I'd be writing again. And then I come
    right back to the DRY principle, and realize I'd refactor myself right
    back to where I am (or something very close to it).

    This is the kind of thing that happens when developers work alone,
    without the benefit of peers to consult.

    And yeah, I'm bitter about it. :)
    Mike
     
    Mike Hofer, Apr 6, 2007
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Muhammad Khan
    Replies:
    4
    Views:
    1,228
    Mike Treseler
    Jul 10, 2003
  2. Replies:
    3
    Views:
    511
    Malcolm
    Sep 29, 2005
  3. rashmi
    Replies:
    2
    Views:
    461
    Grumble
    Jul 5, 2005
  4. Replies:
    3
    Views:
    427
    Malcolm
    Sep 29, 2005
  5. Replies:
    4
    Views:
    663
    Malcolm
    Sep 29, 2005
Loading...

Share This Page