Abstract repository implementation

Question

I have a repository abstract class that encapsulates pretty much all of the CRUD functionality:

public abstract class DataRepository<T> : IRepository<T>
where T : class
{
    public DataContext Context { get; private set; }
    public TransactionScope Transaction { get; private set; }

    /// <summary>
    /// A <see cref="bool"/> function that compares the keys for fetching a single item, for example: 
    /// return item1.Id == item2.Id (as an anonymous delegate).
    /// </summary>
    public Func<T, T, bool> KeyCompare { get; private set; }

    /// <summary>
    /// Creates a new data repository.
    /// </summary>
    /// <param name="context"></param>
    /// <param name="scope"></param>
    /// <param name="keyCompare">
    /// A <see cref="bool"/> function that compares the keys for fetching a single item, for example: 
    /// return item1.Id == item2.Id (as an anonymous delegate).
    /// </param>
    public DataRepository(DataContext context, TransactionScope scope, Func<T, T, bool> keyCompare)
    {
        Context = context;
        Transaction = scope;
        KeyCompare = keyCompare;
    }

    public virtual T Item(T item)
    {
        return Items().SingleOrDefault(e => KeyCompare(e, item));
    }

    public virtual IEnumerable<T> Items()
    {
        return DataTable.AsEnumerable();
    }

    protected virtual Table<T> DataTable { get { return Context.GetTable<T>(); } }

    /// <summary>
    /// A method that updates the non-key fields of an existing entity with those of specified <see cref="item"/>.
    /// Called by the <see cref="Save"/> method.
    /// </summary>
    /// <param name="existing">The existing record to update.</param>
    /// <param name="item">A <see cref="T"/> object containing the values to update <see cref="existing"/> object with.</param>
    /// <returns></returns>
    protected abstract void UpdateExisting(T existing, T item);

    /// <summary>
    /// A method that updates an existing item or creates a new one, as needed.
    /// </summary>
    /// <param name="item">The entity containing the values to be saved.</param>
    public virtual void Save(T item)
    {
        var existing = Item(item);
        if (existing != null)
        {
            UpdateExisting(existing, item);
        }
        else
        {
            DataTable.InsertOnSubmit(item);
        }

        Context.SubmitChanges();
    }

    /// <summary>
    /// A method that saves all specified items (creates new, updates existing).
    /// </summary>
    /// <param name="items">The entities to be saved.</param>
    public virtual void Save(IEnumerable<T> items)
    {
        foreach (var item in items)
        {
            Save(item);
        }
    }

    /// <summary>
    /// A method that deletes specified item.
    /// </summary>
    /// <param name="item"></param>
    public virtual void Delete(T item)
    {
        var existing = Item(item);
        if (existing != null)
        {
            DataTable.DeleteOnSubmit(existing);
        }

        Context.SubmitChanges();
    }

    public virtual void Delete(IEnumerable<T> items)
    {
        var selection = Items().Where(e => items.Any(item => KeyCompare(e, item)));
        DataTable.DeleteAllOnSubmit(selection);

        Context.SubmitChanges();
    }
}

The KeyCompare property is used like this in the derived classes, so that the base class knows how to isolate a single item in the repository (not all "entities" have a "Id" property, and some keys span multiple columns - this solution attempts to resolve that particular point):

public AuthInfoRepository(DataContext context, TransactionScope scope)
    : base(context, scope, (item1, item2) => { return item1.Id == item2.Id;})
{ }

This KeyCompare property is really the cornerstone that allows the derived classes to merely implement the UpdateExisting method, like this:

protected override void UpdateExisting(AuthInfo existing, AuthInfo item)
{
    existing.AuthId = item.AuthId;
    existing.ActiveDirectoryGroup = item.ActiveDirectoryGroup;
}

The rest (actual CRUD) is all handled by the base class. With this abstract repository I have been implementing the concrete ones in minutes if not seconds, writing only the code that is specific to each implementation. Finest DRY I've ever written.

The DataRepository deals with SQL Server, so I needed yet another implementation for mocking, which I've called ListRepository and does pretty much exactly the same thing (except Context and Transaction properties both return null). I think the constructor's signature is all I need to post here:

public ListRepository(IEnumerable<T> items, Func<T, T, bool> keyCompare)

So for testing, I can bind IReporitory<T> to this ListRepository while production code binds to DataRepository:

Bind<IRepository<AuthInfo>>().To<ListRepository<AuthInfo>>()
    .WithConstructorArgument("items", _mockAuthInfo)
    .WithConstructorArgument("keyCompare", (Func<AuthInfo, AuthInfo, bool>)((item1, item2) => item1.Id == item2.Id));

While this is all great (just about to start unit testing it), there are a couple points that I'd still like to address:

• Should I externalize the calls to Context.SubmitChanges()? I don't like that the method gets called at every iteration, both when saving and when deleting multiple items.
• While this is certainly great DRY, I'm worried the virtual methods violate YAGNI - I don't see me overriding any of those in any implmentation in the near future. Then the class doesn't need to be abstract and could still provide a virtual implementation of UpdateExisting, but that would make implementing the class less obvious than the abstract method. Where's the line drawn?

Answer 1

Repositories for

• Add objects
• Remove objects
• Query objects (no named queries like GetById)

Repositories may not know about anything their context or transaction scope (and they can't be saved directly) these are different story which can be told by an UnitOfWork instance:

class UnitOfWork : IDisposable
{
    public void Dispose() {} // Rollback
    public void Commit() {}
}

Table

The Table property ends up with a leaky abstraction and not all repository need a concrete table.

KeyCompare

Nice idea but it will not work as expected becouse it's a Func<> and it can not be translated for example into a SQL query. You should return an Expression<Func<>> and you can apply that as a simple method call no need other expression becouse it will not be translated and maybe you will end up with an exception:

.SingleOrDefault(KeyCompare);

AsEnumerable()

public virtual IEnumerable<T> Items()
    {
        return DataTable.AsEnumerable();
    }

Imagine a reporitory with 20 billion entities which are stored in a SQL database. What will the above expression mean: download all entities and then query them somehow. Avoid calling the .AsEnumerable an similar methods (.ToArray()) before the final query statement.

Items()

God why? The reporitory should represent what you tried with this method but not an IEnumerable<T> but an IQueryable<T> so a repository must implement this interface. But in this way you cold only query your entities but not add/remove them. Need other methods in a common interface but before creating our own check out the IObjectSet<T> interface from the framework it's perfect for a repository.