"JiangZemin" <fourpill***@example.com> wrote in message
news:et7xW2fNFHA.3076@tk2msftngp13.phx.gbl...
> Hi, i wanted to get this groups opinions on a design issue ive run into:
> a programmer has coded a common data access class which is supposed to be
> inherited by all data access layers at my firm.  This class instantiates a
> connection in the constructor, relies heavily on OleDb objects by
> necessity, and does stuff like returns DataReaders and creates DataSets
> and Command objects.
> Before using this class i looked thru the methods and noticed that there
> is no exception handling, so whenever error occurs, open connections and
> OleDb DataReaders and command objects never get closed/disposed.
> Being a helpful fool, i suggested to this programmer that it would be good
> idea to ensure that these methods clean up unmanaged resources that they
> create per best practice.  The response i got is that "you dont understand
> the design", this decision was intentional...  all code that calls this
> classes methods must write their own code to cleanup these resources in
> event of an error.   He did not want to "restrict users of the class from
> flexibility in how they handle errors".
>
> Given that:
> 1. No other programmers who use this class were aware of this necessity

"Miha Markic [MVP C#]" <miha at rthand com> wrote in message
news:e9wUqCgNFHA.3624@TK2MSFTNGP10.phx.gbl...
> hi Jiang,
>
> "JiangZemin" <fourpill***@example.com> wrote in message
> news:et7xW2fNFHA.3076@tk2msftngp13.phx.gbl...
>> Hi, i wanted to get this groups opinions on a design issue ive run into:
>> a programmer has coded a common data access class which is supposed to be
>> inherited by all data access layers at my firm.  This class instantiates
>> a connection in the constructor, relies heavily on OleDb objects by
>> necessity, and does stuff like returns DataReaders and creates DataSets
>> and Command objects.
>> Before using this class i looked thru the methods and noticed that there
>> is no exception handling, so whenever error occurs, open connections and
>> OleDb DataReaders and command objects never get closed/disposed.
>> Being a helpful fool, i suggested to this programmer that it would be
>> good idea to ensure that these methods clean up unmanaged resources that
>> they create per best practice.  The response i got is that "you dont
>> understand the design", this decision was intentional...  all code that
>> calls this classes methods must write their own code to cleanup these
>> resources in event of an error.   He did not want to "restrict users of
>> the class from flexibility in how they handle errors".
>>
>> Given that:
>> 1. No other programmers who use this class were aware of this necessity
>
> This is bad. It should be clearly stated or known who is responsible.

>> 2. the programmer who wrote this class himself never checks for errors
>> from code that calls these methods
>
> Depends on the philosophy.
>
>> 3. its not possible for others to cleanup resources created within those
>> methods
>
> This is horrible :-). Really, if it is required by clients to handle
> exceptions then it should be possible for them to clean up.
>
>> 4. you can still pass exceptions to the caller and still dispose
>> resources you create.
>
> Not exactly. For example, imagine that you return a datareader and caller
> does the reading. How would creator know when the error occurs.

>> 5. there are numerous problems with applications in production which use
>> this data access class.
>
> No wonder :-)
>>
>> my impression is that instead of accepting that a (fairly common)
>> mistake/oversight was made and quickly taking basic steps to correct it,
>> this programmer has decided to defend the indefensible.   What do you
>> think
>
> Mr. Premier JiangZemin :-)
>
> I think that the responsability of closing/disposing resources should be
> of the same instance that creates them. But you can go other ways as long
> as everything is known and well documented (not that I suggest it). In
> your case the bigest mistake is no knowledge of what's going on and how to
> handle errors..
> Since I can't see the actual code I can't comment more than this..

> --
> Miha Markic [MVP C#] - RightHand .NET consulting & development
> www.rthand.com
> SLODUG - Slovene Developer Users Group www.codezone-si.info
>

"JiangZemin" <fourpill***@example.com> wrote in message
news:ebU$pOgNFHA.3988@tk2msftngp13.phx.gbl...
> Hi Miha,
>
> "Miha Markic [MVP C#]" <miha at rthand com> wrote in message
> news:e9wUqCgNFHA.3624@TK2MSFTNGP10.phx.gbl...
>> hi Jiang,
>>
>> "JiangZemin" <fourpill***@example.com> wrote in message
>> news:et7xW2fNFHA.3076@tk2msftngp13.phx.gbl...
>>> Hi, i wanted to get this groups opinions on a design issue ive run into:
>>> a programmer has coded a common data access class which is supposed to
>>> be inherited by all data access layers at my firm.  This class
>>> instantiates a connection in the constructor, relies heavily on OleDb
>>> objects by necessity, and does stuff like returns DataReaders and
>>> creates DataSets and Command objects.
>>> Before using this class i looked thru the methods and noticed that there
>>> is no exception handling, so whenever error occurs, open connections and
>>> OleDb DataReaders and command objects never get closed/disposed.
>>> Being a helpful fool, i suggested to this programmer that it would be
>>> good idea to ensure that these methods clean up unmanaged resources that
>>> they create per best practice.  The response i got is that "you dont
>>> understand the design", this decision was intentional...  all code that
>>> calls this classes methods must write their own code to cleanup these
>>> resources in event of an error.   He did not want to "restrict users of
>>> the class from flexibility in how they handle errors".
>>>
>>> Given that:
>>> 1. No other programmers who use this class were aware of this necessity
>>
>> This is bad. It should be clearly stated or known who is responsible.
>
> Unfortunately even the sample code which this programmer created on how to
> properly use his class does not do this.
>
>>> 2. the programmer who wrote this class himself never checks for errors
>>> from code that calls these methods
>>
>> Depends on the philosophy.
>>
>>> 3. its not possible for others to cleanup resources created within those
>>> methods
>>
>> This is horrible :-). Really, if it is required by clients to handle
>> exceptions then it should be possible for them to clean up.
>>
>>> 4. you can still pass exceptions to the caller and still dispose
>>> resources you create.
>>
>> Not exactly. For example, imagine that you return a datareader and caller
>> does the reading. How would creator know when the error occurs.
>
> Yes, but i meant if the exception happened within the method itself.
>
>>> 5. there are numerous problems with applications in production which use
>>> this data access class.
>>
>> No wonder :-)
>>>
>>> my impression is that instead of accepting that a (fairly common)
>>> mistake/oversight was made and quickly taking basic steps to correct it,
>>> this programmer has decided to defend the indefensible.   What do you
>>> think
>>
>> Mr. Premier JiangZemin :-)
>>
>> I think that the responsability of closing/disposing resources should be
>> of the same instance that creates them. But you can go other ways as long
>> as everything is known and well documented (not that I suggest it). In
>> your case the bigest mistake is no knowledge of what's going on and how
>> to handle errors..
>> Since I can't see the actual code I can't comment more than this..
>
> Understoond, unfortunately this is a one-sided presentation im giving
> here. Its actually more of an opportunity for me to vent about this.
> Thanks for listening!
> -Premier JiangZemin
>
>> --
>> Miha Markic [MVP C#] - RightHand .NET consulting & development
>> www.rthand.com
>> SLODUG - Slovene Developer Users Group www.codezone-si.info
>>
>
>