From: Tom Shelton on
On 2010-02-03, Mr. Arnold <Arnold(a)Arnold.com> wrote:
> JohnS wrote:
>> Hi there,
>>
>> I'm currently using the following pattern in various place but just
>> realized it's probably not safe. Can someone comment on this:
>>
>> public DataRow GetWhatever()
>> {
>> using (DataTable dataTable = GetDataTable())
>> {
>> return dataTable[0];
>> }
>> }
>>
>> The problem is that "DataRow" has a "Table" property that points back to
>> the same "dataTable" in the "using" statement above. That object is
>> disposed of in the "using" statement however so I assume the "Table"
>> property will now point to garbage (or rather an object that shouldn't
>> be used anymore). Is this correct? Note BTW that the indexer above is
>> actually present in my case (the "DataTable" is actually a "DataTable"
>> derivative created using the VS dataset designer)
>
> The real problem is you did a return in the middle of the using
> statement, so the using statement has been short circuited.
>
> The return should be after the using statement.
>
> It should be like this.
>
> public DataRow GetWhatever()
> {
> var dr = new DataRow();
>
> using (DataTable dataTable = GetDataTable())
> {
> dr = dataTable[0];
> }
>
> return dr;
> }

Ummmm... No. it will not short circuit the using - the object dataTable will
always be disposed....

Simple example:

using System;

namespace ConsoleApplication66
{
class Program
{
static void Main ( string[] args )
{
SomeMethod ();
}

static void SomeMethod ()
{
Console.WriteLine ( "Entering Using" );
using ( ADisposableObject a = new ADisposableObject () )
{
Console.WriteLine ( "Returning..." );
return;
}
}
}

class ADisposableObject : IDisposable
{


public void Dispose ()
{
Console.WriteLine ( "Bye! Bye!" );
}


}

}

output:

Entering Using
Returning...
Bye! Bye!

Maybe you meant something different?

--
Tom Shelton
From: Peter Duniho on
Mr. Arnold wrote:
> The real problem is you did a return in the middle of the using
> statement, so the using statement has been short circuited.

Completely false.

> The return should be after the using statement.

There's no difference between having the return statement in the "using"
or outside.

> It should be like this.
>
> public DataRow GetWhatever()
> {
> var dr = new DataRow();
>
> using (DataTable dataTable = GetDataTable())
> {
> dr = dataTable[0];
> }
>
> return dr;
> }

That does exactly what the original code does. The object is still
disposed, and if it was invalided by having its parent object (the
DataTable) disposed in the original example, then it is still
invalidated by having its parent object disposed in your example.

Pete
From: Peter Duniho on
Peter Duniho wrote:
> [...]
> As for the specific example, again´┐Ż the correct solution depends on the
> lifetime of the object. If the lifetime of the object need not extend
> beyond the lifetime of the caller, then the caller can simply use a
> "using" statement to manage that:
>
> public void DoWhicever()
> {
> using (DataRow row = GetWhatever())
> {
> // do stuff
> }
> }

Sorry, I neglected to make the code example address the specific example
you actually had. That was silly.

The original disposable object in your example is of course the entire
DataTable. So, you have two options: delegate management of the
lifetime of the DataTable object to the caller, and let it deal with
extracting a specific row; or, clone the data from the DataTable in some
useful way, and pass the clone back so that the DataTable itself can be
disposed of in the called method.

Which is the best approach depends entirely on the rest of the design.
For example, where the DataTable instance came from and how the
underlying data structures related to it are being managed. But in
general, the latter approach will involve the least modification to the
overall code base, of course.

Pete
From: JohnS on
> Sorry, I neglected to make the code example address the specific example
> you actually had. That was silly.
>
> The original disposable object in your example is of course the entire
> DataTable. So, you have two options: delegate management of the lifetime
> of the DataTable object to the caller, and let it deal with extracting a
> specific row; or, clone the data from the DataTable in some useful way,
> and pass the clone back so that the DataTable itself can be disposed of in
> the called method.
>
> Which is the best approach depends entirely on the rest of the design. For
> example, where the DataTable instance came from and how the underlying
> data structures related to it are being managed. But in general, the
> latter approach will involve the least modification to the overall code
> base, of course.

Thanks again for your insight. Unfortunately it just doesn't work out very
well in this case. First, "DataRow" doesn't inherit from "IDisposable" which
puts the onus on the user to explicitly call "DataRow.Table.Dispose()"
(instead of putting a "using" statement around "DataRow"). This is hardly
elegant to say the least and certainly error-prone to expect undisciplined
programmers to constantly do this (just the reality). Cloning or copying
tables of data is also grossly inefficient for a DB application in
particular and probably even more problematic in this case, since I would
have to clone types that are created by the Visual Studio forms designer
itself. Not sure if this is doable at all off-hand - would have too look
into it but it almost certainly means more overhead when less is always
better. Returning the "DataTable" object itself is also syntactically ugly
when the table contains just one row in these cases (used for key searches,
etc.). It's always the first and only row in the table users are after so
returning the row is much cleaner. I'm resigned at this point to not worry
about disposing of these "DataTable" objects for now since AFAIK, they
consume no unmanaged resources I probably have to worry about. That is, they
pick up "IDisposable" further up the hierarchy and various discussions about
it on the web seem to confirm this. It's ultimately a trade-off however
between pedantic adherence to proper "IDisposable" rules (which I'd prefer
to follow), vs the trouble it's causing in this case.

From: Mr. Arnold on
JohnS wrote:

> Cloning or copying tables of data is also grossly
> inefficient for a DB application in particular and probably even more
> problematic in this case, since I would have to clone types that are
> created by the Visual Studio forms designer itself.

A good database application should have an abstraction layer away from
the database, which makes for a more scalable and workable solution. The
problem is the usage of outdated technology -- datatables.

The second thing is not disconnecting from the data source via another
object.