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.

Negotiation

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 :-)

Essequibo

I like your suggestion on using a non-abstract Repository to handle the common low-level CRUD-type work, delegated from the specific repos. It does raise the issue of getting an ISession into both the generic repo and the specific repo using it. – Negotiation

The ISession should be obtained via a private ISessionFactory in the Repository. The specific Repository then uses a WrapxxxInSession method of the generic Repository. – Essequibo

Ah yes, that helps. I quite like the overall idea...another benefit is that the injected IRepository can be mocked for testing. There will be the trick of constructing and passing objects in the right order, but likely an IoC tool can handle that. – Negotiation

It can - I've used both a ServiceLocator pattern as well as an IoC container to help in that department. – Essequibo

Answer 2

1. try not to pass back an array. use IEnumerable<T>, ICollection<T> or IList<T>, this will loosely couple your code further.

2. your IMovieRepository interface. this repository includes the CRUD. therefore make it

IMovieRepository : IRepository<Movie> {}

This will not change your MovieRepository class as that will implement the interface correctly. it will allow you to decouple your classes if you wish to change the implementation at a later date.

finally. this is fine for one of the methods. as you have specialised functionality you have specialised the repository to suit.

there are other ways, which enable you to use 1 repositry class and pass in the required query. This is called the Specification pattern. I did a project which uses this located on codeplex with report http://whiteboardchat.codeplex.com

the other way would to to have a method to pass in the criteria. there is a open source project called Sharp Architecture, which i believe has this coded up.

Hope this helps

Logorrhea

Thanks for the pointers on the Specification pattern implementation, and the criteria-based methods. Both are interesting ideas, and I will look closer at them, though right now I'm keeping things simple and straightforward. – Negotiation

Answer 3

As food for thought, if your ORM of choice has a LINQ provider (and NH has one) you can try the queryable repository that is quite similar to a collection:

public interface IRepository<T> : ICollection<T>, IQueryable<T>

I have written some about it on my site: Repository or DAO?: Repository It has similarities to your construction (just because a collection supports CRUD as well), the approach I tries means that you can have code that isn't necessarily aware of dealing with a repository as it can be programmed against the ICollection or IQueryable interface...

Dichloride