closing connection not by design

  • Thread starter Thread starter JiangZemin
  • Start date Start date
J

JiangZemin

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
2. the programmer who wrote this class himself never checks for errors from
code that calls these methods
3. its not possible for others to cleanup resources created within those
methods
4. you can still pass exceptions to the caller and still dispose resources
you create.
5. there are numerous problems with applications in production which use
this data access class.

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?

Premier JiangZemin
 
hi Jiang,

JiangZemin said:
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
Click to expand...

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
Click to expand...

Depends on the philosophy.
3. its not possible for others to cleanup resources created within those
methods
Click to expand...

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.
Click to expand...

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.
Click to expand...

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
Click to expand...

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..
 
Hi Miha,

Miha Markic said:
hi Jiang,



This is bad. It should be clearly stated or known who is responsible.
Click to expand...

Unfortunately even the sample code which this programmer created on how to
properly use his class does not do this.
Depends on the philosophy.


This is horrible :-). Really, if it is required by clients to handle
exceptions then it should be possible for them to clean up.


Not exactly. For example, imagine that you return a datareader and caller
does the reading. How would creator know when the error occurs.
Click to expand...

Yes, but i meant if the exception happened within the method itself.
No wonder :-)

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..
Click to expand...

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
 
I'd be venting by way of firing the, obviously, incompetent programmer and
also firing the idiot who approved the specification for the common data
access class or allowed the incompetent programmer to design the common data
access class himself.

Such people can only be a liability to your organisation.
 
Back
Top