Should I use generics to simplify my DAL?

Question

I'm new to NHibernate and not very good at C#, but I'm learning. I have a DataProvider class which provides data for my application using NHibernate 3. It's structured pretty much identical to Steve Bohlen's Summer of NHibernate videos.

I've noticed that I'm about to repeat my code a lot and I want to simplify my DataProvider. For example I have two business classes called Instrument and Broker. The method to add an Instrument in my DataProvider is:

    public int AddInstrument(Instrument instrument)
    {
        using (ITransaction tx = _session.BeginTransaction())
        {
            try
            {
                int newId = (int)_session.Save(instrument);
                _session.Flush();
                tx.Commit();
                return newId;
            }
            catch (NHibernate.HibernateException)
            {
                tx.Rollback();
                throw;
            }
        }
    }

and the AddBroker class looks remarkably similar (just find and replace). So I thought maybe I could use Generics to solve the problem. Something like:

public class DataProvider <TEntity>
{
    public int AddEntity(TEntity entity)
    {
        using (ITransaction tx = _session.BeginTransaction())
        {
            try
            {
                int newId = (int)_session.Save(entity);
                _session.Flush();
                tx.Commit();
                return newId;
            }
            catch (NHibernate.HibernateException)
            {
                tx.Rollback();
                throw;
            }
        }
    }    
}

With this I can pass in a Broker, an Instrument or anything and I save myself a lot of repetitive code. The problem I'm having is that in my Test class I create a new DataProvider each time a test is run like this:

    #region Fields

    private DataProvider _provider;
    private SessionManager _sessionManager;
    private NHibernate.ISession _session;

    #endregion

    [SetUp]
    public void Setup()
    {
        DatabaseSetUp();

        _session = _sessionManager.GetSession();
        _provider = new DataProvider(_session);    // problem here
    }

Using generics I have to pass in the object type to my DataProvider. Can you think of a way to solve this? I am a novice programmer and I am wondering if I am going down the right path. Should I be doing something totally different?

UPDATE

I have tried to implement Groo's answer but am having some issues. Here's what I've done.

IRepo.cs

interface IRepo<T>
{
    int Add<Entity>(Entity entity);
    void Delete<Entity>(Entity entity);
    void GetById<Entity>(int Id);
}

BaseRepo.cs

public abstract class BaseRepo <T> : IRepo <T>
{
    private ISession _session;

    #region SessionManagement

    public BaseRepo(ISession session)
    {
        _session = session;
    }

    public ISession Session
    {
        set { _session = value; }
    }

    #endregion

    public int Add<Entity>(Entity entity)
    {
        using (ITransaction tx = _session.BeginTransaction())
        {
            try
            {
                int newId = (int)_session.Save(entity);
                _session.Flush();
                tx.Commit();
                return newId;
            }
            catch (NHibernate.HibernateException)
            {
                tx.Rollback();
                throw;
            }
        }
    }

    // other methods omitted for brevity
}

IRepoFactory.cs

interface IRepoFactory
{
    IInstrumentRepo CreateInstrumentRepo(ISession s);
}

RepoFactory.cs

public class RepoFactory : IRepoFactory
{
    public IInstrumentRepo CreateInstrumentRepo(ISession s) // problem here
    {
        return new InstrumentRepo(s);
    }

}

IInstrumentRepo.cs

interface IInstrumentRepo : IRepo<Instrument>
{

}

InstrumentRepo.cs

public class InstrumentRepo : BaseRepo<Instrument>, IInstrumentRepo
{
    public InstrumentRepo(ISession s) : base(s) { }
}

In RepoFactory.cs I get this error:

Inconsistent accessibility: return type 'MooDB.Data.IInstrumentRepo' is less accessible than method 'MooDB.Data.RepoFactory.CreateInstrumentRepo(NHibernate.ISession)'

Any ideas what I'm missing?

Answer 1

First of all, to address your Test Setup question: the term Repository might suggest that it should be a long lived, persistent object, but Repositories used in DAL operations should actually be lightweight stateless objects with short lifetimes: you instantiate one when you need it, and throw it away as soon as you're done. When you think about this is terms of performance, you can easily instantiate millions of them per second.

Combined with NHibernate's short lived Session instances, this is how your code is meant to look like when everything is in place:

using (var session = SessionManager.OpenSession())
{
    // create an instrument repo
    IInstrumentRepo instruments = DAL.RepoFactory.CreateInstrumentRepo(session);
    var guitar = instruments.Find(i => i.Type == "Guitar");

    // create a customer repo
    ICustomerRepo customers = DAL.RepoFactory.CreateCustomerRepo(session);
    var cust = customers.Find(c => c.Name == "Mark")

    // do something -> changes will be persisted by NH when session is disposed
    cust.Instruments.Add(guitar);
}

That's the general idea. Now, let me explain it in more detail:

1. You may have noticed that each repo has its own interface, and is created through a repo factory. Using a factory to create repositories means you can easily create mock repo factories, which will create any custom implementation of a repository for testing.

2. Each repo interface inherits from the base interface generic interface, IRepo<T>. This allows you to use a generic repository in 99% of cases, but still leave room to implement a custom query method specific to, say, Customer entities only:

   public interface IInstrumentRepo : IRepo<Instrument>
   { 
       // nothing to do here
   }
   
   public interface ICustomerRepo : IRepo<Customer>
   {
       // but we'll need a custom method here
       void FindByAddress(string address);
   }
   
   public interface IRepo<T> 
   {
       T GetById(object id);
       T Save(T item);
   }
   

3. This means that your repo implementations will, in most cases, simply inherit from the base abstract class (I named it BaseRepo, but it's essentially what your DataProvider class does right now):

   class InstrumentRepo : BaseRepo<Instrument>, IInstrumentRepo
   {
       // no need to implement anything here except pass the session downwards
       public InstrumentRepo(ISession s) : base(s) { }
   }
   

4. Your factory will simply need to instantiate the proper repository when asked:

   public class RepoFactory : IRepoFactory
   {
        public IInstrumentRepo CreateInstrumentRepo(ISession s)
        {
            return new InstumentRepo(s);
        }
   }
   

5. And you will need to use the Singleton pattern in a, say, DAL class to hold the factory (there are slightly better ways to do this, using DI, but for now this will do just fine):

   public static class DAL 
   {
       // repo factory is pretty lightweight, so no need for fancy
       // singleton patterns
   
       private static readonly IRepoFactory _repoFactory = new RepoFactory();
       public static IRepoFactory RepoFactory
       { 
           get { return _repoFactory; }
       }
   }
   

Answer 2

The answer to your question is Absolutely Yes! This is what generics are meant for.

You are in the right way.

This argument is really too long to discuss here but you can find a lot of usefull info in this article:

http://www.codeproject.com/KB/architecture/NHibernateBestPractices.aspx

It helps me a lot to create my generic nhibernate Dao

Answer 3

Your data provider class doesn't necessarily need to be generic - you can just make the AddEntity method itself generic. Then you instantiate a DataProvider instance, and call (for example) its AddEntity<Instrument> method. Your class would look like this:

public class DataProvider
{
    public int AddEntity<TEntity>(TEntity entity)
    {
        using (ITransaction tx = _session.BeginTransaction())
        {
            try
            {
                int newId = (int)_session.Save(entity);
                _session.Flush();
                tx.Commit();
                return newId;
            }
            catch (NHibernate.HibernateException)
            {
                tx.Rollback();
                throw;
            }
        }
    }    
}