Generic UnitOfWork implementation (Dapper)

Question

I'm creating a three layered Visual Studio template solution that I'll be using as a starting point for all ASP.NET projects. The layers are Presentation, Application and Infrastructure. IUnitOfWork and IRepository are declared in the Application layer. UnitOfWork is implemented in the Infrastructure layer and it's injected through dependency injection container.

public interface IUnitOfWork : IDisposable
{
 IDbTransaction Transaction { get; }
 TRepository? Repository<TRepository>() where TRepository : class;
 void Commit();
 void Rollback();
 void AddRepository(IRepository repository);
}
public interface IRepository
{
 void SetTransaction(IDbTransaction transaction);
}

public class UnitOfWork : IUnitOfWork
{
 private readonly IDbConnection _connection;
 private readonly IServiceProvider _serviceProvider;
 private IDbTransaction _transaction;
 private bool _disposed;
 private readonly List<IRepository> _repositories = new();
 public IDbTransaction Transaction => _transaction;
 public UnitOfWork(IDbConnection dbConnection, IServiceProvider serviceProvider)
 {
 _connection = dbConnection;
 if (_connection.State != ConnectionState.Open) _connection.Open();
 _transaction = _connection.BeginTransaction();
 _serviceProvider = serviceProvider;
 }
 public TRepository? Repository<TRepository>() where TRepository : class
 {
 var repository = _serviceProvider.GetService<TRepository>();
 if (repository is IRepository transactionAware)
 {
 transactionAware.SetTransaction(_transaction);
 AddRepository(transactionAware);
 }
 return repository;
 }
 public void Commit()
 {
 try
 {
 _transaction.Commit();
 }
 catch
 {
 throw;
 }
 finally
 {
 UpdateTransaction();
 }
 }
 public void Rollback()
 {
 _transaction.Rollback();
 UpdateTransaction();
 }
 private void UpdateTransaction()
 {
 _transaction = _connection.BeginTransaction();
 foreach (var repository in _repositories)
 {
 if (repository is IRepository transactionAware)
 {
 transactionAware.SetTransaction(_transaction);
 }
 }
 }
 public void Dispose()
 {
 if (!_disposed)
 {
 _transaction?.Rollback();
 _transaction?.Dispose();
 _connection?.Dispose();
 _disposed = true;
 }
 }
 public void AddRepository(IRepository repository)
 {
 if (_repositories.Contains(repository)) return;
 _repositories.Add(repository);
 }
}

services.AddScoped<IDbConnection>(provider =>
{
 var connectionString = configuration.GetConnectionString("DefaultConnection");
 return new NpgsqlConnection(connectionString);
});
services.AddScoped<IUnitOfWork, UnitOfWork.UnitOfWork>();

Usage

public class MyService
{
 private readonly IMyRepository _myRepository;
 private readonly IUnitOfWork _unitOfWork;
 public MyService(IUnitOfWork unitOfWork)
 {
 _unitOfWork = unitOfWork;
 _myRepository = unitOfWork.Repository<IMyRepository>();
 }
}

Answer 1

constructor

Perform null checks against the parameters to make sure that you won't have unexpected problem later. Like for instance if the serviceProvider is null then it won't break at the constructor rather at the Repository() method.

The constructor has too many responsibilities. It should only perform parameter checks and parameter - field mapping. Create a dedicated method to perform the database communication.

Commit method

This catch is just a noise. It does not represent any value that's the default behavior.
You can use try-finally block without catch

public void Commit()
{
 try
 {
 _transaction.Commit();
 }
 finally
 {
 UpdateTransaction();
 }
}

Repository method

If the repository does not implement the IRepository then you should throw an exception like InvalidOperationException rather than returning the class without indicating the issue.

In C# the name of the methods usually start with a verb. Please try to follow C# guidelines.

Dispose method

This is a naive implementation of IDispose interface. Please check Microsoft recommendation how to implement it properly.

AddRepository method

Please bear in mind that this implementation is not thread safe. Either use a lock object whenever you manipulate the underlying _repositories or use one of the concurrent collections, like ConcurrentDictionary.

Consider Async support

Since database communication is an I/O heavy operation with battle tested async support it make sense to provide async support for Commit and Rollback methods.

• Rick Davin: Combining 2 of your useful suggestions, if OP implements Async support, then DisposeAsync should be implemented as well. Good points all around.
• FAMO4S: I'm thankful for your suggested corrections. I tried to implement them and posted a follow-up question. Could you please take a look at it? I didn't implement DisposeAsync because it seemed redundant to me.