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.

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();
    }
}