Unit of Work (UoW) pattern with ADO.NET

Question

I'm trying to implement the UoW pattern using ADO.NET and this is what I've achieved so far:

The IUnitOfWork Interface:

public interface IUnitOfWork : IDisposable
{
 IDomainTableRepository DomainTables { get; } 
 IVariableRepository Variables { get; }
 IModelRepository Models { get; } 
 IStructureRepository Structures { get; }
 ISentenceRepository Sentences { get; }
 IExpressionRepository Expressions { get; }
 IReturnRepository Returns { get; }
 void Commit();
}

The implementation for IUnitOfWork:

public class SqlUnitOfWork : IUnitOfWork
 {
 const string ConnectionStringName = "DefaultConnection"; 
 private AdoNetContext _context;
 private DomainTableRepository _domainTables;
 private VariableRepository _variables; 
 private ModelRepository _models; 
 private StructureRepository _structures; 
 private SentenceRepository _sentences;
 private ExpressionRepository _expressions; 
 private ReturnRepository _returns; 
 public SqlUnitOfWork()
 {
 var connectionString =
 ConfigurationManager
 .ConnectionStrings[ConnectionStringName]
 .ConnectionString;
 _context = new AdoNetContext(connectionString, true); 
 } 
 public IDomainTableRepository DomainTables
 {
 get
 {
 if (_domainTables == null)
 {
 _domainTables = new DomainTableRepository(_context);
 }
 return _domainTables;
 }
 } 
 //...getters for the remaining repositories 
 public void Commit()
 {
 _context.SaveChanges();
 }
 #region IDisposable Support
 private bool disposedValue = false; 
 protected virtual void Dispose(bool disposing)
 {
 if (!disposedValue)
 {
 if (disposing)
 {
 _context.Dispose();
 } 
 disposedValue = true;
 }
 } 
 public void Dispose()
 { 
 Dispose(true); 
 }
 #endregion
 }

The AdoNetContext class

All the credit for the implementation of this class goes to @jgauffin.

His original blog post can be found here.

public class AdoNetContext : IDisposable
 {
 private IDbConnection _connection;
 private bool _ownsConnection;
 private IDbTransaction _transaction;
 public AdoNetContext(string connectionString, bool ownsConnection)
 {
 _connection = new SqlConnection(connectionString);
 _connection.Open();
 _ownsConnection = ownsConnection;
 _transaction = _connection.BeginTransaction();
 } 
 public IDbCommand CreateCommand()
 { 
 var command = _connection.CreateCommand();
 command.Transaction = _transaction;
 return command;
 } 
 public void SaveChanges()
 {
 if (_transaction == null)
 {
 throw new InvalidOperationException("Transaction have already been already been commited. Check your transaction handling.");
 }
 _transaction.Commit();
 _transaction = null;
 } 
 public void Dispose()
 {
 if (_transaction != null)
 {
 _transaction.Rollback();
 _transaction = null;
 } 
 if (_connection != null && _ownsConnection)
 {
 _connection.Close();
 _connection = null;
 }
 }
 }

The implementation of a repository (DomainTableRepository) that uses AdoNetContext:

public class DomainTableRepository : IDomainTableRepository
 {
 private AdoNetContext _context; 
 public DomainTableRepository(AdoNetContext context)
 {
 _context = context;
 } 
 public DomainTable Get(int id)
 {
 DomainTable table;
 using (var commandTable = _context.CreateCommand())
 {
 commandTable.CommandType = CommandType.StoredProcedure;
 commandTable.CommandText = "up_DomainTable_GetById";
 commandTable.Parameters.Add(commandTable.CreateParameter("@pId", id));
 table = ToList(commandTable).FirstOrDefault();
 }
 return table;
 } 
 public IEnumerable<DomainTable> GetAll(int pageIndex = 1, int pageSize = 10)
 { 
 using (var command = _context.CreateCommand())
 {
 command.CommandType = CommandType.StoredProcedure;
 command.CommandText = "up_DomainTable_GetAll";
 command.Parameters.Add(command.CreateParameter("@pPageIndex", pageIndex));
 command.Parameters.Add(command.CreateParameter("@pPageSize", pageSize)); 
 return ToList(command).ToList();
 } 
 } 
 public int Remove(DomainTable entity)
 {
 using (var command = _context.CreateCommand())
 {
 command.CommandType = CommandType.StoredProcedure;
 command.CommandText = "up_DomainTable_Delete";
 command.Parameters.Add(command.CreateParameter("@pId", entity.Id));
 return command.ExecuteNonQuery();
 }
 } 
 }

This is how I want to use the UOW in my controllers (using constructor injection):

 public class DomainTableController : Controller
 {
 private IUnitOfWork _unitOfWork;
 public DomainTableController(IUnitOfWork unitOfWork)
 {
 _unitOfWork = unitOfWork;
 }
 public ActionResult Index()
 {
 var model = _unitOfWork.DomainTables.GetAll();
 return View(model);
 }
 // other actions ...
 }

This is the only piece of code that I added to the file that was automatically created in App_Start when I added the Ninject package:

 public static class NinjectWebCommon
 {
 // other methods
 private static void RegisterServices(IKernel kernel)
 {
 //Obviously this line is not working 
 //as the SqlUniOfWork's Dispose method is never called
 kernel.Bind<IUnitOfWork>().To<SqlUnitOfWork>();
 }
 }

First, I created an interface for the UoW (IUnitOfWork), because in the future I might want to change to an ORM like EF, but since I'm just taking my first step into the the world of back-end programming I wanted to start with ADO.NET and stored procedures.

Here are my questions regarding the implementation of the UoW pattern with ADO.NET and its usage in ASP.NET MVC:

1. Given the code for the AdoNetContext (the one that manages the db connection and transactions), is there any performance penalty for always creating a transaction?

   This is what I read in a SQL book:

   Executing a SELECT statement within a transaction can create locks on the referenced tables, which can in turn block other users or sessions from performing work or reading data"

   Do the transactions created in C# behave exactly as their counterparts in SQL?

2. As you can see in IUnitOfWork and its implementation (UnitOfWork), I have readonly properties for every repository I have in my application. This a common "pattern" I've seen in lots of tutorials on EF and I find it very convenient since every time I new up a instance of IUnitOfWork I already have access to all its repositories and I don't have to clutter my code with the instantiation of the repositories I need in a specific Action (from a Controller).

   Do you think it would be too much overhead to instantiate all the repositories (as SQLUnitOfWork does) every time I create/inject a new instance of IUnitOfWork? There are some scenarios in which I might work with 4 or 5 repositories at the same time, but in most cases I'll be using just 1 or 2 in the same action.

3. I would like to use DI (maybe Ninject) to inject an instance of IUnitOfWork into my Controllers. This way later when I use another persistence framework, I will only have to change this line kernel.Bind<IUnitOfWork>().To<SqlUnitOfWork>(); From SqlUnitOfWork, to let's say EFUnitOfWork.

   The problem I've encountered here is that the Dispose methods in SqlUnitOfWork and AdoNetContext are never called and as much as I want to use DI I'd rather wrap the instantiation of SqlUnitOfWork in an using statement in every Action instead of be leaking memory just so that I can use DI.

• Mathieu Guindon: You haven't shown how you're injecting the UoW in a controller, nor whether you've set up MVC's ControllerFactory to lay controller instantiation in Ninject's hands, but kernel.Bind<IUnitOfWork>().To<SqlUnitOfWork>() will bind your UoW in transient scope, which does not get disposed by default; you'll want to bind it .InRequestScope() instead, and then I don't see why .Dispose() wouldn't get called. Don't do DI halfway, you'll lose the advantages of DI (e.g. how will you mock your UoW in controller tests if you new up the UoW and disposing it in each controller action?)
• eddy: @Mat'sMug Thank you for taking the time to comment. Regarding your question about the ControllerFactoryI don't know what to say about it since this is the first time I heard of it :( . I did try to use InRequestScope(), but it didn't work either, the only time I was able to verify that the Dispose method was actually called was when I instantiated the UoW within a using statement, maybe due to the piece of configuration you mentioned. Btw, I edited my question to add a example of how I want to inject the UoW in my controllers.
• Mathieu Guindon: I don't do much web dev, but IIRC if you just install the ninject.extensions.mvc through nuget it takes care of everything for you. Read up on that extension if you're not using it yet.
• eddy: I see :( What about my other two questions (1 and 2) ? I think they fall under the categories that you mostly review , according to your profile ;)
• Mathieu Guindon: I don't think a transaction is going to be a bottleneck. As for the repos it doesn't hurt if nothing significant happens in the ctors. =)

Answer 1

you could collapse this piece of code into a single line

 public IDomainTableRepository DomainTables
 {
 get
 {
 if (_domainTables == null)
 {
 _domainTables = new DomainTableRepository(_context);
 }
 return _domainTables;
 }
 }

by using a Null Coalescing operator (??)

like this

public IDomainTableRepository DomainTables
{
 get
 {
 return _domainTables ?? new DomainTableRepository(_context);
 }
}

unfortunately this isn't quite the same as what you have because it doesn't perform "Lazy Initialization" but the following code, that @Programmer gave in a comment, does so while also keeping with the spirit of using less indentation

public IDomainTableRepository DomainTables => 
 _domainTables ?? (_domainTables = new DomainTableRepository(_context));

I also noticed that in one instance you put the return statement outside of the using block but in the rest of the methods in that class you put the returns inside of the using blocks, while either will work you should stay consistent with the way that you write your code.

the code that I am talking about

 public DomainTable Get(int id)
 {
 DomainTable table;
 using (var commandTable = _context.CreateCommand())
 {
 commandTable.CommandType = CommandType.StoredProcedure;
 commandTable.CommandText = "up_DomainTable_GetById";
 commandTable.Parameters.Add(commandTable.CreateParameter("@pId", id));
 table = ToList(commandTable).FirstOrDefault();
 }
 return table;
 }

and while I am here I would even place the Declaration of the variable inside of the using statement because there isn't anything about it (that I can tell) that would make it mandatory to declare it outside of the using statement

public DomainTable Get(int id)
{
 using (var commandTable = _context.CreateCommand())
 {
 DomainTable table;
 commandTable.CommandType = CommandType.StoredProcedure;
 commandTable.CommandText = "up_DomainTable_GetById";
 commandTable.Parameters.Add(commandTable.CreateParameter("@pId", id));
 table = ToList(commandTable).FirstOrDefault();
 return table;
 }
} 

You could actually just return the results of the ToList Method call like this:

public DomainTable Get(int id)
{
 using (var commandTable = _context.CreateCommand())
 {
 commandTable.CommandType = CommandType.StoredProcedure;
 commandTable.CommandText = "up_DomainTable_GetById";
 commandTable.Parameters.Add(commandTable.CreateParameter("@pId", id));
 return ToList(commandTable).FirstOrDefault();
 }
} 

• programmer: The use of ?? doesn't ever set the _domainTables field so it's not the same behavior as OPs.
• programmer: setting the value in the get method is lazy initialization and it's identical to how Scott Allen on Pluralsight shows how to add repositories in a Unit of Work class. I've used this pattern before and was able to unit test without touching the database. One issue this might have is if your repository classes start having many deps, but that shouldn't happen if the repositories have proper responsibilities. Also, this one liner provides the same behavior public IDomainTableRepository DomainTables => _domainTables ?? (_domainTables = new DomainTableRepository(_context));
• Malachi: Mind if I steal your One Liner, @programmer, your argument is very valid. Thank you for that bit of knowledge.
• programmer: Go ahead and steal it @Malachi, I stole it from someone else :)
• Malachi: good catch @dfhwze