WPF/MVVM with Entity Framework, Repository and Unit of Work pattern

Question

I'm building WPF/MVVM application and I'm struggling implementing Unit of work pattern.

The part, about which I'm not sure is UnitOfWork class. Here is my thought:

It's coupled with my repositories and MyDatabaseContext. At first impression, it doesn't sound good. But, I think, UnitOfWork should be coupled with DbContext. Technically it should be named MyDatabaseUnitOfWork. What do you think?

Unit of work:

public interface IUnitOfWork : IDisposable 
{
 int Complete();
 ICustomerRepository Customers { get; }
 IPhoneRepository Phones { get; }
 IAddressRepository Address { get; }
}
public class UnitOfWork<TContext> : IUnitOfWork
 where TContext : MyDatabaseContext, new()
{
 public ICustomerRepository Customers { get; }
 public IPhoneRepository Phones { get; }
 public IAddressRepository Address { get; }
 public UnitOfWork()
 {
 context = new TContext();
 Customers = new CustomerRepository(context);
 Phones = new PhoneRepository(context);
 Address = new AddressRepository(context);
 } 
 public int Complete()
 {
 return context.SaveChanges();
 }
 public void Dispose()
 {
 context.Dispose();
 }
}
public interface IUnitOfWorkFactory
{
 IUnitOfWork Create();
}
public class UnitOfWorkFactory : IUnitOfWorkFactory
{
 public IUnitOfWork Create()
 {
 return new UnitOfWork<MyDatabaseContext>();
 }
}

Useage:

public partial class App : Application
{
 protected override void OnStartup(StartupEventArgs e)
 {
 base.OnStartup(e);
 try
 {
 UnityContainer unityContainer = new UnityContainer();
 unityContainer.RegisterType<IUnitOfWorkFactory, UnitOfWorkFactory>();
 unityContainer.RegisterType<MainWindowViewModel>();
 var window = new MainWindow()
 {
 DataContext = unityContainer.Resolve<MainWindowViewModel>()
 };
 window.Show();
 }
 catch (Exception ex)
 {
 MessageBox.Show(ex.Message);
 }
 }
}
public class MainWindowViewModel : ObservableObject
{
 private readonly IUnitOfWorkFactory unitOfWorkFactory;
 public MainWindowViewModel(IUnitOfWorkFactory uowFactory)
 {
 unitOfWorkFactory = uowFactory;
 }
 private int DoSomething()
 {
 using (var unitOfWork = unitOfWorkFactory.Create())
 {
 }
 }
}

Sample repository:

public interface IRepository<TEntity> where TEntity : class
{
 TEntity Get(int id);
 IEnumerable<TEntity> Get(Func<TEntity, bool> predicate);
 IEnumerable<TEntity> GetAll();
 void Add(TEntity entity);
 void AddRange(IEnumerable<TEntity> entities);
 void Remove(TEntity entity);
 void RemoveRange(IEnumerable<TEntity> entities);
}
public interface ICustomerRepository : IRepository<Customer>
{
 IEnumerable<Customer> GetTopCustomers(int count);
}
internal class CustomerRepository : Repository<Customer>, ICustomerRepository
{
 private MyDatabaseContext MyDatabaseContext => Context as MyDatabaseContext;
 internal CustomerRepository(MyDatabaseContext context)
 : base(context)
 {
 }
 public IEnumerable<Customer> GetTopCustomers(int count)
 {
 return MyDatabaseContext.Customers.ToList();
 }
}

---

Here is my second option: Is it necessary?

public interface IUnitOfWork : IDisposable 
{
 int Complete();
}
public interface IMyDatabaseUnitOfWork : IUnitOfWork 
{
 int Complete();
 ICustomerRepository Customers { get; }
 IPhoneRepository Phones { get; }
 IAddressRepository Address { get; }
}
public class MyDatabaseUnitOfWork<TContext> : IMyDatabaseUnitOfWork
 where TContext : MyDatabaseContext, new()
{
 public ICustomerRepository Customers { get; }
 public IPhoneRepository Phones { get; }
 public IAddressRepository Address { get; }
 public MyDatabaseUnitOfWork()
 {
 context = new TContext();
 Customers = new CustomerRepository(context);
 Phones = new PhoneRepository(context);
 Address = new AddressRepository(context);
 } 
 public int Complete()
 {
 return context.SaveChanges();
 }
 public void Dispose()
 {
 context.Dispose();
 }
}

• Mattias Åslund: EF already implements the pattern Unit of Work. Does your class solve a real problem for you or is it just xomplicating things?
• Marshall: @MattiasÅslund Well, in my "second option", my UnitOfWork is completely decoupled from EF or any other ORM/Service or whatever, isn't it? So my viewmodel or business layer doesn't know about EF or any other ORM.
• Adriano Repetti: @Marshall Mattia's point is: do you need it in this moment or in a reasonable near future? If not then you're writing tons of unnecessary code...
• Adriano Repetti: @abuzittingillifirca much more than that. Whole repository pattern is useless more often than not with EF (unless you're planning to use different ORMs). All presented code is unnecessary (and it's still far away to be complete), not even mention that it's working all in-memory (Func<> instead of Expression<> and ToList()) then probably it's a fictional example. Now, given the amount of required code to write (few thousands line - when half properly done and still missing some EF features - compared to zero) with the advantage of...hmmm well which advantage?
• Adriano Repetti: Of course calling code should be updated if this intermediate layer is removed...you can read it as "all presented code - with the exception of calling code - is unnecessary". I'm not thinking about missing essential features he WILL need. I'm exactly against this because it's a feature he does not need now!!! Fictional examples are 50 lines of code, final implementation is...how much? thousands lines? What's the immediate (or near future) benefit? Please note that I'm not an XP adept but unnecessary layers and complexity always make me ask "why" (he may have reasons)

Answer 1

You should consider using loosely coupled code. IoC / DI are a great way to go. Example:

public class ImmutableUnitOfWork
{
 private readonly ICustomerRepository _customers;
 private readonly IPhoneRepository _phones;
 private readonly IAddressRepository _address;
 public ImmutableUnitOfWork(
 ICustomerRepository customers,
 IPhoneRepository phones,
 IAddressRepository address)
 {
 _customers = customers;
 _phones = phones;
 _address = address;
 } 
}

using StructureMap your dependency resolution should look something in the lines of:

 ObjectFactory.Initialize(x =>
 {
 x.Scan(scan =>
 {
 scan.TheCallingAssembly();
 scan.WithDefaultConventions();
 scan.AssemblyContainingType<ICustomerRepository>(); // Core
 scan.AssemblyContainingType<CustomerRepository>(); // Infrastructure
 scan.AssemblyContainingType<MyDatabaseContext>(); // Just in case 
 });
 x.For(typeof(IRepository<>)).Use(typeof(Repository<>));
 // WithDefaultConventions basically says if you name interfaces with
 // same name, but with "I" prefix you do not need following:
 x.For(typeof(IMyDatabaseContext)).Use(typeof(MyDatabaseContext));
 x.For(typeof(ICustomerRepository)).Use(typeof(CustomerRepository));
 });

but I understand if it is for learning purposes.

---

From code perspective :

ICustomerRepository is a service, not a repository. IRepository<TEntity> - is leaky, especially IEnumerable<TEntity> Get(Func<TEntity, bool> predicate);. If you feel that you really need it, try using specification pattern instead. This is just bad on many levels:

 catch (Exception ex)
 {
 MessageBox.Show(ex.Message);
 }

• Margus: Your code BL code would go inside ImmutableUnitOfWork class.