Return DataReader From Method; Good or Bad Practice?

[Jeff S]

I want to instantiate my DAL class within a presentation-layer class, and
have methods in the the DAL return a DataReader. I have seen a number of
posts in this newsgroup that recommended one to never return a DataReader
from a method. The reasoning given is to prevent memory leaks. Sounds
good... but in the ASP.NET Starter Kits - which we are to use as examples
for how to do things - that very practice shows up a lot.

Here's an example right out of the Commerce starter kit:
public SqlDataReader GetProductCategories() {
....
}

Is this practice safe (i.e., won't create memory leaks) if/when we specify
CommandBehavior.CloseConnection as in the following line?
SqlDataReader result =
myCommand.ExecuteReader(CommandBehavior.CloseConnection);
return result;

Thoughts? Opinions? Facts?

Thanks!

 

[Steve C. Orr [MVP, MCSD]]

I really think this is a matter of opinion. Personally I don't usually
return DataReaders from methods for the reasons you mention.
But I've had lively debates with peers about the merits of the technique.
There was no clear winner in these debates.
If you do choose to return DataReaders, it is imperative that they get
closed as soon as possible. Do whatever you can to make sure this rule is
followed. If you only use this technique privately within your own class
libraries then you can have absolute control over it.

 

[Jeff S]

Thanks

BTW: What's the upshot of your cart/frames project from last week? (just
curious what you've concluded now that some time has passed).

(Jeff/Dewek)

Agreed that it's handy to return a datareader sometimes from a generic

"getdatareader" routine. As long as it's set to .closeconnection, when you
close the datareader that will take care of the connection, and as Steve
says if you're doing that all within your class you have control over the
housekeeping. A datareader is more efficient than caching to a dataset, so
if all you're trying to do is, say, fill up a dropdownlist and don't need to
hang onto the data source (i.e. you're happy letting the page hold onto the
items collection in the viewstate), then I'd use a datareader,

 

[Guest]

I've put off any bold decisions there. All the pieces I care about are in user controls and pretty self-sufficient, so now that I understand both sides reasonably well going frame/no frames, I'll make that decision later (or perhaps provide an option for both), and for now am off to some trickier bits.

 

[bruce barker]

but did you put a try/catch/finally around the data bind, with a finally
statement to close the reader in case of an error condition? did you have a
try/catch/finally around the request for reader until you were done with it?
do all exit paths close the reader?

I find most resource leaks are from not handling cleanup on errors. for this
reason I don't think a DAL should loan out resouces it can not control. also
the connection is tried up for however long the caller needs until its
processed the reader completely. and horrors of horrors maybe the caller
threw the datareader into a Session variable


-- bruce (sqlwork.com)

Agreed that it's handy to return a datareader sometimes from a generic

"getdatareader" routine. As long as it's set to .closeconnection, when you
close the datareader that will take care of the connection, and as Steve
says if you're doing that all within your class you have control over the
housekeeping. A datareader is more efficient than caching to a dataset, so
if all you're trying to do is, say, fill up a dropdownlist and don't need to
hang onto the data source (i.e. you're happy letting the page hold onto the
items collection in the viewstate), then I'd use a datareader,

 

[Jeff S]

What's your take on CommandBehavior.CloseConnection?

Does it actually do nothing? Perhaps I'm just naive on this, but it seems
like that would take care of the closing the connection if the caller closes
the data reader.

I understand if I'm creating a commercial product or a class that's going to
get used by unknown developers that I can't assume that they are going to do
the right thing - but I'm specifically asking about the scenario in which
the DAL and its consumers are all within my happy little
application/assembly and are not exposed to any idiots who would place a
DataReader into the Session. Yes, I could screw up and forget to close
something - but IMHO that's hardly reason to forego having any methods at
all return DataReaders. But I'm still in the process of forming my opinion
on this and would appreciate your take on CommandBehavior.CloseConnection
and whether it actually mitigates any of the legitimate issues you raised.

Thanks!

 

[Guest]

..closeconnection works for what it's supposed to do; the point is that if you have untrapped errors and you don't hit datareader.close that you have a problem

Bruce, I'm curious, in the simple case of filling a listbox, would your DAL return a dataset, or maybe XML? And if that's the case, do you think the cleanliness of having the DAL not leave anything to chance with its users is worth the expense of staging the data? Or, is there some other clever way to get the benefits of streaming the data forward-only but without leaving the connection hanging

----- Jeff S wrote: ----

What's your take on CommandBehavior.CloseConnection

Does it actually do nothing? Perhaps I'm just naive on this, but it seem
like that would take care of the closing the connection if the caller close
the data reader

I understand if I'm creating a commercial product or a class that's going t
get used by unknown developers that I can't assume that they are going to d
the right thing - but I'm specifically asking about the scenario in whic
the DAL and its consumers are all within my happy littl
application/assembly and are not exposed to any idiots who would place
DataReader into the Session. Yes, I could screw up and forget to clos
something - but IMHO that's hardly reason to forego having any methods a
all return DataReaders. But I'm still in the process of forming my opinio
on this and would appreciate your take on CommandBehavior.CloseConnectio
and whether it actually mitigates any of the legitimate issues you raised

Thanks

 

[Ravichandran J.V.]

Functionally, a DAL should be returning a DataSet rather than a
DataReader because the layer's functionality is to provide data and to
have a connected access scenario may seem like bad practice. But then,
it is also according to preferences at times.

with regards,


J.V.Ravichandran
- http://www.geocities.com/
jvravichandran
- http://www.411asp.net/func/search?
qry=Ravichandran+J.V.&cob=aspnetpro
- http://www.southasianoutlook.com
- http://www.MSDNAA.Net
- http://www.csharphelp.com
- http://www.poetry.com/Publications/
display.asp?ID=P3966388&BN=999&PN=2
- Or, just search on "J.V.Ravichandran"
at http://www.Google.com