Repository wrapper

Question

The following implementation is of a repository proxy.

I will post only the code for NHibernate Repository here. Everything else (including configurers and tests) can be found on the pastebin.

P.S. : I changed all the XML-like comments to their more readable representation, so the actual code still has nice XML documentation.

public interface IRepository
{
    // Retrieves every entity of the specified type stored in the repository.
    IQueryable<T> RetrieveEntities<T>() where T : IRepositoryEntity;
    // Retrieves every entity filtered by the supplied expression.
    // As long as the result is models 'IQueryable', it implies on the lazy result
    // evaluation and therefore doesn't have significant performance impact.
    IQueryable<T> RetrieveEntities<T>(Expression<Func<T, bool>> expression) where T : IRepositoryEntity;

    // New items are added to the repository and existing are updated.
    void AddEntities<T>(IQueryable<T> sequence) where T : IRepositoryEntity;
    void AddEntities<T>(params T[] entities) where T : IRepositoryEntity;

    // Removes every entity of the specified type stored in the repository.
    void RemoveEntities<T>() where T : IRepositoryEntity;
    // Removes entities of the specified type which fit under the specified expression.
    void RemoveEntities<T>(Expression<Func<T, bool>> expression) where T : IRepositoryEntity;
    // Removes every entity in the sequence from the repository. Throws if at least one entity
    // from the sequence doesn't belong to the database.
    void RemoveEntities<T>(IQueryable<T> sequence) where T : IRepositoryEntity;
    void RemoveEntities<T>(params T[] entities) where T : IRepositoryEntity;
}

---

public class NHibernateRepository : IRepository
{
    private readonly Configuration configuration;
    private readonly ISessionFactory sessionFactory;
    private readonly ISession session;

    public NHibernateRepository(NHibernateRepositoryConfigurer repositoryConfigurer)
    {
        // Build and store the NHibernate-specific configuration.
        configuration = repositoryConfigurer.Configuration.BuildConfiguration();

        // Build the corresponding session factory and open the session.
        sessionFactory = repositoryConfigurer.SessionFactory;
        session = sessionFactory.OpenSession();
    }

    public IQueryable<T> RetrieveEntities<T>() where T : IRepositoryEntity
    {
        CheckTypeMappings(typeof(T));
        return session.Query<T>();
    }

    public IQueryable<T> RetrieveEntities<T>(Expression<Func<T, bool>> expression) where T : IRepositoryEntity
    {
        CheckTypeMappings(typeof(T));
        return session.Query<T>().Where(expression);
    }

    public void AddEntities<T>(IQueryable<T> sequence) where T : IRepositoryEntity
    {
        CheckTypeMappings(typeof(T));
        WithinTransaction(() =>
        {
            foreach (var entity in sequence)
            {
                session.Merge(entity);
            }
        });
    }

    public void AddEntities<T>(params T[] entities) where T : IRepositoryEntity
    {
        CheckTypeMappings(typeof(T));
        AddEntities(entities.AsQueryable());
    }

    public void RemoveEntities<T>() where T : IRepositoryEntity
    {
        CheckTypeMappings(typeof(T));
        WithinTransaction(() =>
        {
            foreach (var entity in session.Query<T>())
            {
                session.Delete(entity);
            }
        });
    }

    public void RemoveEntities<T>(Expression<Func<T, bool>> expression) where T : IRepositoryEntity
    {
        CheckTypeMappings(typeof(T));
        WithinTransaction(() =>
        {
            foreach (var entity in session.Query<T>().Where(expression))
            {
                session.Delete(entity);
            }
        });
    }

    public void RemoveEntities<T>(IQueryable<T> sequence) where T : IRepositoryEntity
    {
        CheckTypeMappings(typeof(T));
        WithinTransaction(() =>
        {
            foreach (var entity in sequence)
            {
                session.Delete(entity);
            }
        });
    }

    public void RemoveEntities<T>(params T[] entities) where T : IRepositoryEntity
    {
        CheckTypeMappings(typeof(T));
        RemoveEntities(entities.AsQueryable());
    }

    // Performs the entire specified action within a single unit of work.
    private void WithinTransaction(Action action)
    {
        var transaction = session.BeginTransaction();

        try
        {
            action();
            transaction.Commit();
        }
        catch (Exception)
        {
            transaction.Rollback();
            throw;
        }
        finally
        {
            transaction.Dispose();
        }
    }

    // Throws if the type is not mapped to the database.
    private void CheckTypeMappings(Type type)
    {
        if (type.IsInterface) return;

        if (configuration.ClassMappings.Any(x => x.MappedClass == type)) return;

        // The 'type' is definitely doesn't have an appropriate persister.
        throw new MappingException("Type " + type.FullName + " doesn't have an appropriate persister");
    }

Answer 1

Two things i notied:

Wrap the transaction in a using-statement and also log the exception.

 // Performs the entire specified action within a single unit of work.
    private void WithinTransaction(Action action)
    {
        using(var transaction = session.BeginTransaction()){

           try
           {
               action();
               transaction.Commit();
           }
           catch (Exception ex)
           {
               transaction.Rollback();
               _logger.ErrorFormat("Error description here... {0}", ex.Message);
               throw;
           }
           finally
           {
               transaction.Dispose();
           }
        }
    }

And also you should make the interface generic instead of each method (unless there is a reason for that). Makes sense and results in higher readability.

 public interface IRepository<T> where T : IRepositoryEntity
    {
        IQueryable<T> RetrieveEntities<T>();
        IQueryable<T> RetrieveEntities<T>(Expression<Func<T, bool>> expression);  

        void AddEntities<T>(IQueryable<T> sequence);
        void AddEntities<T>(params T[] entities); 

        void RemoveEntities<T>();
        void RemoveEntities<T>(Expression<Func<T, bool>> expression);
        void RemoveEntities<T>(IQueryable<T> sequence);
        void RemoveEntities<T>(params T[] entities);
    }