While my assumption may seem to sound subjective, after some research, I found that it's not uncommon to find developers who favour a dummy Try/Catch
instead of using the Using statement for IDbConnection/IDbTransaction
processing (Close/Commit/Rollback).
This holds true for even some of the most seasoned developers and some new ones. I am intentionally not going to reference any of the question on StackOverflow or forum links as an example, so people don't get offended. From what I found, Using statement is safe to use (no pun intended).
Is there anything wrong with it? Consider the following code:
Public Sub Commit()
Dim cn As IDbConnection = {CREATE_CONNECTION}
Dim tran As IDbTransaction = Nothing
cn.Open()
Try
tran = cn.BeginTransaction
'run some queries here
tran.Commit()
Catch ex As Exception
If Not tran Is Nothing Then tran.Rollback()
Throw
Finally
cn.Close()
End Try
End Function
Assume {CREATE_CONNECTION}
is place holder for a Sub
that creates a connection, depending on the database vendor, written according to all possible best practices and does not need more improvement.
Is there a reason why the above code cannot be rewritten as such:
Using cn As IDbConnection = {CREATE_CONNECTION}
cn.Open()
Using tran As IDbTransaction = cn.BeginTransaction
'run some queries here
tran.Commit()
End Using
End Using
?
Clearly, version #2 is more intuitive to what it's doing. But perhaps I am missing something important here? Things like vendor-specific implementations of data access libraries, that do not call Transaction.Commit
and/or Connection.Close
on Dispose
internally? Is this approach being decommissioned in the near future, or not regarded as clear enough in modern programming pattern/best practices? Mono/mobile apps dev tools lacking debug support for Using
keyword?
I am looking for any kind of answer to support or deny the point. Preferably the one with quotes to original documentation, something like Do not use Using with IDbTransaction when ...
. Links to blogs or personal experience are okay too.
I'm entirely with you on the connection; that should be using
, and there is no need for the explicit Close()
. The transaction is a little bit trickier; the code shown is certainly overkill at the moment, but it is not entirely defined that Dispose()
should do a rollback. Actually, that is what tends to happen in every implementation I've looked at, but it is slightly vexing that even DbTransaction
(which most providers use) doesn't actually do this. Contrast to TransactionScope
where it is explicitly defined that a Dispose()
without a commit counts as a rollback. Because of that, I tend to use (excuse the C#):
using(var conn = GetOpenConnection())
using(var tran = conn.BeginTransaction()) {
try {
// TODO: do work
tran.Commit();
} catch {
tran.Rollback();
throw;
}
}
which is somewhere between the two in terms of complexity. It isn't messing around with null
-checks, at least.