I'd like to write a simple unit of work class that would behave like this:
using (var unitOfWork = new UnitOfWork())
{
// Call the data access module and don't worry about transactions.
// Let the Unit of Work open a session, begin a transaction and then commit it.
}
This is what I have so far (any comments will be welcome if you think my design is wrong):
class UnitOfWork : IDisposable
{
ISession _session;
ITransation _transaction;
.
.
.
void Dispose()
{
_transaction.Commit();
_session.Dispose();
}
}
What I would like to do is to roll back the transaction in case the data acess code threw some exception. So the Dispose()
method would look something like:
void Dispose()
{
if (Dispose was called because an exception was thrown)
{
_transaction.Commit();
}
else
{
_transaction.RollBack();
}
_session.Dispose();
}
Does it make sense? And if so, how can it be done?
The "Dispose()" should have nothing to do with transaction commit or rollback. You should dispose you transaction in your Dispose() method. Changing the semantics of your Dispose() method will only add to confusion in the long run for you and anyone else using your class.
The Transaction's Commit() and RollBack() methods have nothing whatsoever to do with a Dispose() method since there is no correlation between these two method and Dispose(), since you have to dispose a transaction no matter what the final outcome is.
This is the correct pattern to use as regards connections and transactions. Notice that Roolback(0 relates to an exception (and not dispose)
connection.Open();
var trasnaction = null;
try
{
transaction = connection.BeginTransaction();
///Do Some work
transaction.Commit();
}
catch
{
transaction.Rollback();
}
finally
{
if (transaction != null)
transaction.Dispose();
connection.Close();
}
So mimic this pattern in your UnitOfWork, with methods for Commit(), Roolback() and Dispose().
A bit late to the game here, but check this post from Ayende for a (slightly mad) solution:
In the Dispose method, you just need to figure out whether or not it got there 'cleanly' - i.e. figure out if an unhandled exception exists before committing the transaction:
public class ExceptionDetector : IDisposable
{
public void Dispose()
{
if (Marshal.GetExceptionCode()==0)
Console.WriteLine("Completed Successfully!");
else
Console.WriteLine("Exception!");
}
}
You need to UnitOfWork.Commit()
at the end of the using
block. Inside UnitOfWork
you have a committed
flag that you check in UnitOfWork.Dispose
. If the flag is false
there, then you UnitOfWork.Rollback()
.
The point of Dispose
is that it is always run. And by using this commit-rollback idiom you don't need to know the difference.
using (var unitOfWork = new UnitOfWork())
{
// use unitOfWork here - No need to worry about transactions for this code.
unitOfWork.Commit();
}
Here we see that either an exception is thrown or the unitOfWork is committed. We can then have a bool in UnitOfWork
keeping track whether the Commit was executed or not. Then the Dispose
can Rollback not commited. This way the unit-of-work is always either rollbacked or commited.
I would in any case avoid having the Commit inside the Dispose. For starters a ITransaction.Commit
method typically may throw exceptions on errors - This is perfectly normal. However, the Dispose
method is not supposed to throw exceptions. See this link and search on Stackoverflow for more indepth information on why.
I'm thinking something like this in big strokes
class UnitOfWork : IDisposable
{
ISession _session;
ITransation _transaction;
bool _commitTried;
// stuff goes here
void Commit()
{
_commitTried = true;
_transaction.Commit();
}
void Dispose()
{
if (!_commitTried) _transaction.Rollback();
_transaction.Dispose();
_session.Dispose();
}
}
The problem with forgetting to call Commit altogether I would say is not that big since unless commited, the client code will not work since the transaction is Rollback
'ed and changes are not applied which will be discovered when exercising the code inside a fixture or manually.
I actually tried to deal with this in one project by using a syntax with lambdas like so
_repository.InTransactionDo(ThisMethodIsRunInsideATransaction);
This way clients did not need to worry about Commiting anything. I actually ended up regretting this since it complicated things too much and wished I hade gone with the above approach.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With