Am I using IRepository correctly?

Question

I'm looking to use the IRepository pattern (backed by NHibernate, if it matters) in a small project. The domain is a simple one, intentionally so to allow me to focus on understanding the IRepository pattern. The lone domain class is Movie, with properties for Year, Genre, and Title. My intent would be to "get" movies whose properties match criteria of the aforementioned types.

Convention seems to be to have a generic IRepository interface, similar to the following:

public interface IRepository<T>
{
    T Get(int id);
    T[] GetAll();
    void Add(T item);
    void Update(T item);
    void Delete(T item);
}

With a base implementation:

public abstract class Repository<T> : IRepository<T>
{
    public T Get(int id) { ... }
    public T[] GetAll() { ... }
    public void Add(T item) { ... }
    public void Update(T item) { ... }
    public void Delete(T item) { ... }
}

Then to have a domain-specific interface:

public interface IMovieRepository
{
    Movie[] GetByGenre(Genre genre);
    Movie[] GetByYear(int year);
    Movie[] GetByTitle(string title);
}

With an implementation that also extends the base Repository class:

public class MovieRepository : Repository<Movie>, IMovieRepository
{
    public Movie[] GetByGenre(Genre genre) { ... }
    public Movie[] GetByYear(int year) { ... }
    public Movie[] GetByTitle(string title) { ... }
}

I would need to add necessary implementation to the base class as well as the concrete one, using NHibernate, but I would like to know if I am on the right track with this setup.

There seems to be a fair bit of overhead for just one domain class, though it would be less noticeable if there were multiple domain classes involved. Right now I'm trying to keep it simple so I can pin down the concept.

Answer 1

I'd say, you are close to the repository that I use in a production solution for resource planning in Transport companies (using NHibernate as well) - so for starters you are on the right path in my opinion. I agree with dbones on using IEnumerables /IList instead of arrays - you'll end up writing .ToArray() many times over :-).

A few things you might consider:

Favour Composition over inheritance - instead of inheriting from the abstract repository - let it be non-abstract and inject it in the 'ctor and delegate the calls - this makes your design more robust in certain situations (e.g. for a Query-only repository etc.) That way you also have the option of letting the abstract repository be instantiatable (is that a word?) and control whether it should be shared across all repositories.

Following up on that point - you might want to change the base Repository to have generic methods instead of inheriting from the generic interface:

public class Repository
{
    public void Add<T>(T entity)
    {
        using(var session = GetSession())
        using(var tx = session.BeginTransaction())
        {
             session.Save(entity)
             //Transaction handling etc.
        }
    }
    .... //repeat ad nasseum :-)
}

You might want to let the specific repositories have access to the ISession - this greatly improves how flexible you can make your queries and control eager/lazy fetching and you get full advantage of NHibernate etc. E.g.

public class Repository
{
    public IList<T> WrapQueryInSession<T>(Func<ISession,IList<T> query)
    {
        using(var session = GetSession())
        using(var tx = session.BeginTransaction())
        {
             var items = query(session);
             //Handle exceptions transacitons etc.
             return items;
        }
     }
 }

Usage:

public class MovieRepository : IMovieRepository
{
    private Repository _repository;
    public MovieRepository(Repository repository)
    {
        _repository = repository;
    }
    public IList<Movie> GetByYear(int year)
    {
        Func<ISession, IList<Movie> query = session =>
        {
            var query = session.CreateQuery("from Movie"); //or
            var query = session.CreateCriteria("from Movie"); //or
            var query = session.Linq<Movie>();
            //set criteria etc.
            return query.List<Movie>(); //ToList<Movie>() if you're using Linq2NHibernate
        }:
        return _repository.WrapQueryInSession(query);
    }
}

You might also want to set a bool return value on your methods if something goes wrong - and maybe an out IEnumerable for any errors that will make sense in the calling code.

But all in all - these are just my tidbits that I have added over time to comply better with my usage - and they are entirely optional, just food for thought :-). I think you are on the right path - I don't see any major problems in your code.

Hope this makes sense :-)