I recently read about unit of work and repository design patterns; working with EF wondering if this is a good design pattern

Question

I found myself creating child records that require a master record to exist, first, so that they can reference it by the master record's primary key (if that's the right term).

To accomplish this using EF 6 I tried to call SaveChanges() twice - once to create the master record so that its identity key gets generated and once after the children have been created.

The problem I ran into was that EF doesn't like multiple calls to SaveChanges() so I had to use a transaction. I don't like using a transaction because it feels messy for some reason I can't articulate.

It's a somewhat common problem for me to have to do this so instead of a using, try/catch and commit/rollback I figured it'd be easier to have this each time:

this._dbContextWrapper.CommitIfTrue(() =>
{
    // Multiple calls to SaveChanges() here

    return true;
});

While also having BeginTransaction() exposed on my _dbContextWrapper so that if passing a Func<bool> isn't desirable.

public interface IRepository
{
    IQueryable<TModel> Query<TModel>()
        where TModel : class;

    TModel Find<TModel>(params object[] key)
        where TModel : class;

    void Add<TModel>(TModel model)
        where TModel : class;

    void Update<TModel>(TModel updated, params object[] key) 
        where TModel : class;

    IDataResult SaveChanges();

    void Delete<TModel>(params object[] key)
        where TModel : class;

    IRepositoryTransaction BeginTransaction();

    IRepositoryTransaction BeginTransaction(IsolationLevel isolationLevel);

    IDataResult CommitIfTrue(Func<bool> transaction);

    IDataResult CommitIfTrue(Func<bool> transaction, IsolationLevel isolationLevel);
}

public interface IRepositoryTransaction : IDisposable
{  
    void Commit();

    void Rollback();
}

public interface IDataResult
{
    bool IsSuccess { get; }

    string ErrorMessage { get; }
}

I realize that this "IRepository" only a thin wrapper around DbContext - I'm doing it so that my classes can have a Mock implementation provided to them instead of having to mess with Fakes to get around the non-virtual aspect of DbContext's methods.

My question is:

Is my CommitIfTrue method reasonable? Is there a way to do something similar that's also more testable/involves no Actions or Funcs?

Answer 1

I don't know why it would need to return true. Why couldn't it simply commit if no exception is thrown? That's the way it would be if you just had a transaction in a using statement. There are arguments for and against external vs. internal control of these types of things (ie. external vs internal iteration), but I don't see the need to have BOTH CommitIfTrue() and BeginTransaction(). They essentially provide the same functionality.