Dispose() and transactions

  • Thread starter Thread starter Daniel Billingsley
  • Start date Start date
D

Daniel Billingsley

Does this look right? I want to be a good boy and dispose of everything and
I'm trying to make sure I have the lifecycles right to support the
transactional part.

using (SqlConnection cn = new SqlConnection("blah blah")
{
cn.open();
using (SqlTransaction tran = cn.BeginTransaction())
using (SqlCommand cm = new SqlCommand())
{
cm.Connection = cn;
cm.Transaction = tran;
try
{
// do some stuff
cm.ExecutNonQuery();
tran.Commit();
}
catch(Exception ex)
{
tran.RollBack();
throw ex;
}
}
 
umm... how about adding a finally{} block to your try block to close your
connection and set it to null. Right now your not disposing of anything,
your letting GC do it when your ref's are out of scope.

Hope this helps.

Nick Harris, MCP, CNA, MCSD
Director of Software Services
http://www.VizSoft.net
 
Daniel Billingsley said:
Does this look right? I want to be a good boy and dispose of everything and
I'm trying to make sure I have the lifecycles right to support the
transactional part.
Click to expand...

Daniel, this is almost exactly how I do it. Mine is structured a little bit
differently. Also you don't need the ex on the throw. I took your example
and redid it based on the way that I do it.

using (SqlConnection cn = new SqlConnection("blah blah") {
cn.open();
using (SqlTransaction tran = cn.BeginTransaction()) {
try {
using (SqlCommand cm = new SqlCommand()) {
cm.Connection = cn;
cm.Transaction = tran;
// do some stuff
cm.ExecutNonQuery();
tran.Commit();
}
} catch {
tran.RollBack();
throw;
}
}
}

-- Alan
 
Nick Harris said:
umm... how about adding a finally{} block to your try block to close your
connection and set it to null. Right now your not disposing of anything,
your letting GC do it when your ref's are out of scope.
Click to expand...

No - note the "using" blocks, which *do* perform a Dispose implicitly.
The connection, transaction and command are all being disposed. Setting
the variables to null would be unnecessary, too, as they'd fall out of
scope anyway.
 
Alan Pretre said:
Daniel, this is almost exactly how I do it. Mine is structured a little bit
differently. Also you don't need the ex on the throw. I took your example
and redid it based on the way that I do it.
Click to expand...

<snip>

Just as a minor point, I can't see from the docs what happens to a
transaction that hasn't been either committed or rolled back but is
then disposed. It would make sense to my mind for any such transaction
to be rolled back - which would also make the code a lot simpler (you
wouldn't need the extra try block) - what am I missing?

(I suspect this is actually what it does, but as the documentation
doesn't state that, it would be wrong to assume it.)
 
Jon Skeet said:
Just as a minor point, I can't see from the docs what happens to a
transaction that hasn't been either committed or rolled back but is
then disposed. It would make sense to my mind for any such transaction
to be rolled back - which would also make the code a lot simpler (you
wouldn't need the extra try block)
Click to expand...

I'm not sure about the answer to your hypothesis. But at least in my
snippet all of the code paths result in a Commit() or a Rollback(), which
seems to make intent clear.
what am I missing?
Click to expand...

I don't think you're missing anything... would you post your version of the
snippet without the try/catch...

-- Alan
 
Alan Pretre said:
I'm not sure about the answer to your hypothesis. But at least in my
snippet all of the code paths result in a Commit() or a Rollback(), which
seems to make intent clear.
Click to expand...

That's true.
I don't think you're missing anything... would you post your version of the
snippet without the try/catch...
Click to expand...

Sure (although I can't bring myself to use your bracing style, I'm
afraid :)

using (SqlConnection cn = new SqlConnection("blah blah"))
{
cn.Open();
using (SqlTransaction tran = cn.BeginTransaction())
{
using (SqlCommand cm = new SqlCommand())
{
cm.Connection = cn;
cm.Transaction = tran;
// do some stuff
cm.ExecuteNonQuery();
tran.Commit();
}
}
}

If an exception is thrown, the command gets disposed, then the
transaction gets disposed, then the connection gets disposed. When the
transaction is disposed, it could check whether or not it had been
committed, and roll back if not.

I don't know for sure, not having done much database work in .NET, but
I presume that the tran.Commit() could actually be called *after* the
using block with the SqlCommand? (For many transactions you'd want one
command after another, wouldn't you?) If so, I'd put it that way so
that it was more logical: do everything with the command, and then
commit the transaction after you've finished with the command.

The above looks like cleaner code to me, but does rely on the reader
knowing that Dispose would perform that rollback. I suspect that if
SqlTransaction were documented to have that behaviour, it would become
sufficiently ingrained as an idiom that it wouldn't be a problem.
 
Jon Skeet said:
I don't know for sure, not having done much database work in .NET, but
I presume that the tran.Commit() could actually be called *after* the
using block with the SqlCommand? (For many transactions you'd want one
command after another, wouldn't you?) If so, I'd put it that way so
that it was more logical: do everything with the command, and then
commit the transaction after you've finished with the command.
Click to expand...

Agreed. The SqlTransaction object has no dependency on the SqlCommand
object.

So we are left with either (although I can't bring myself to use your
bracing style, I'm
afraid :) :

using (SqlConnection cn = new SqlConnection("blah blah") {
cn.open();
using (SqlTransaction tran = cn.BeginTransaction()) {
try {
using (SqlCommand cm = new SqlCommand()) {
cm.Connection = cn;
cm.Transaction = tran;
// do some stuff
cm.ExecutNonQuery();
}
tran.Commit();
} catch {
tran.RollBack();
throw;
}
}
}

OR

using (SqlConnection cn = new SqlConnection("blah blah") {
cn.open();
using (SqlTransaction tran = cn.BeginTransaction()) {
using (SqlCommand cm = new SqlCommand()) {
cm.Connection = cn;
cm.Transaction = tran;
// do some stuff
cm.ExecutNonQuery();
}
tran.Commit();
}
}

-- Alan
 
Just as a minor point, I can't see from the docs what happens to a
transaction that hasn't been either committed or rolled back but is
then disposed.
Click to expand...

If the transaction is disposed before a Commit(), it's the same as an
implicit vote for a rollback. An explicit call to Commit() is required to
preserve the work performed under the transaction.
 
If the transaction is disposed before a Commit(), it's the same as an
implicit vote for a rollback. An explicit call to Commit() is required to
preserve the work performed under the transaction.
Click to expand...

That was I expected - but do you know whether it's actually
*documented* anywhere? It's the only thing that makes any sense, IMO,
but that's not the same as it being worth depending on. Maybe it'll be
documented in Whidbey...
 
This is one of the things I wanted to verify. You're implying the command
objects don't need to be around for the transaction to commit or rollback
and my code was based on thinking they did. But I think your thinking seems
more logical now that I look at it again. I guess I wasn't sure if a
transaction object is encapsulated with all the information it needs to
commit or rollback (without the commands), but then that would be good OO
and make more sense, eh?

That would certainly have to be the case if you reused the command, changing
the CommandText and doing another ExecuteNonQuery, for example, wouldn't it?
I assume that's permitted.
 
Speaking of bracing style, you don't like my

using (SqlTransaction tran = cn.BeginTransaction())
using (SqlCommand cm = new SqlCommand())
{

code? Maybe I'm too simple-minded, but removing a level of indentation
makes things easier to follow for me. Would you find this code hard to
read, or is it just a tom-a-to/tom-ah-to kind of thing? <Realizing that
analogy may lose something going across the ocean as you may pronounce them
the same...lol>
 
Daniel Billingsley said:
Speaking of bracing style, you don't like my

using (SqlTransaction tran = cn.BeginTransaction())
using (SqlCommand cm = new SqlCommand())
{

code?
Click to expand...

Hey I'm a K&R guy. I'm not in vogue right now. Sooner or later the fad
will come back to my way again. ;-)

-- Alan
 
Daniel Billingsley said:
Speaking of bracing style, you don't like my

using (SqlTransaction tran = cn.BeginTransaction())
using (SqlCommand cm = new SqlCommand())
{

code?
Maybe I'm too simple-minded, but removing a level of indentation
makes things easier to follow for me.
Click to expand...

But you haven't *really* removed a level of indentation - you've just
not shown it, if you see what I mean. You could do the same everywhere
and just not bother indenting at all, but it wouldn't help at all.

The bracing style I was talking about was the "brace at the end" style
though.
Would you find this code hard to read, or is it just a tom-a-to/tom-ah-to
kind of thing?
Click to expand...

I would find it "odd" to read in the same way that I'd find:

if (x==y)
Foo();

hard to read - it doesn't show the indentation I'd expect to see,
namely that the Foo() part is effectively encompassed by the if (x==y)
part.
 
Actually, I was talking more to Jon who used my style except for this
section. You're stuff is, well, just ugly. LOL.
 
Jon Skeet said:
But you haven't *really* removed a level of indentation - you've just
not shown it, if you see what I mean.
Click to expand...

Oh yes, of course, I realize that.
I would find it "odd" to read in the same way that I'd find:

if (x==y)
Foo();

hard to read - it doesn't show the indentation I'd expect to see,
namely that the Foo() part is effectively encompassed by the if (x==y)
part.
Click to expand...

Hmm. Interesting. I think I actually saw that example somewhere, but can't
find it. The Help shows them nested with indentation but no brackets.
using()
using()
...
using()
statement;

Since I'm one that would generally lean towards using brackets even for a
one-line statement for the sake of clarity...
if (x==y)
{
DoSomething();
}

I guess I'm being inconsistent. I was thinking this was a generally
accepted exception since I remembered seeing the example, and I would think
nested using() statements would be fairly common.

Guess I thought wrong. :)
 
Daniel Billingsley said:
Hmm. Interesting. I think I actually saw that example somewhere, but can't
find it. The Help shows them nested with indentation but no brackets.
using()
using()
...
using()
statement;
Click to expand...

Indented with no brackets is certainly better than not indented with no
brackets, but I'd prefer indented *with* brackets :)
Since I'm one that would generally lean towards using brackets even for a
one-line statement for the sake of clarity...
if (x==y)
{
DoSomething();
}
Click to expand...

I'm hypocritical on that one - I think it's a good idea, but I'm often
too lazy to do it, unfortunately :(
I guess I'm being inconsistent. I was thinking this was a generally
accepted exception since I remembered seeing the example, and I would think
nested using() statements would be fairly common.
Click to expand...

I'm sure they're fairly common (especially for cases like this) but
that doesn't mean it's a good idea to miss the braces out - IMO, of
course.
Guess I thought wrong. :)
Click to expand...

Not necessarily - plenty of people disagree with me on all kinds of
matters, so I wouldn't be surprised if this is another one :)
 
Back
Top