0

I have a question about how can manage multiple repositories with unit of work. I thought for two solutions but I'm not sure. The first is a classic solution where the IUnitOfWork interface looks like this:

public interface IUnitOfWork
{
    IUserRepository UserRepository { get; set; }
    IUserSecurity UserSecurityRepository { get; set;}
    IUserHistoryRepository UserHistoryRepository {get; set; }
    void Commit()
}

and its implementations is like this:

public class SqlUnitOfWork : IUnitOfWork
{
    private readonly MyDbContext _dbContext;

    public SqlUnitOfWork(MyDbContext dbContext, IUserRepository userRepository,
    IUserHistoryRepository userHistoryRepository, IUserSecurityRepository userSecurityRepository)
    {
        _dbContext = dbContext;
        UserRepository = userRepository;
        UserHistoryRepository = userHistoryRepository;
        UserSecurityRepository = userSecurityRepository;
    }

    public IUserRepository UserRepository { get; }

    public IUserHistoryRepository UserHistoryRepository { get; }

    public IUserSecurityRepository UserSecurityRepository { get; }

    public void Commit()
    {
        _dbContext.SaveChanges();
    }
}

the second solution I'm thinking of is like this:

public interface IUnitOfWork
{
    IRepository<TEntity> GetRepository<TEntity>() where TEntity : class;
    void Commit()
}

and its implementation is:

public class SqlUnitOfWork : IUnitOfWork
{
    private readonly Container _container;

    public SqlUnitOfWork(Container container)
    {
        _container = container;
    }

    public IRepository<TEntity, TKey> GetRepository<TEntity, TKey>() where TEntity : class
    {
        var repo = _container.GetInstance<IRepository<TEntity, TKey>>();

        if(repo == null)
            throw new ArgumentException("Specified repository is not registered");

        return repo;
    }

    public void Save()
    {
        _container.GetInstance<MyDbContext>().SaveChanges();
    }
}

In both cases I used the SimpleInjector library as a DI library. In the first case, however, I already have 4 injected parameters in the constructor and if I needed to add other user-related repositories (formerly UserProfileRepository, etc.) everything would become unmanageable and CodeSmell The second solution is a little more prestigious, but I used the container as ServiceLocator and it is considered an AntiPattern. What solution should I choose and how could I improve everything to make everything a little more SOLID? ? I hope I was clear. Thanks

  • I'd go with first approach. At first it looks like a pointless code duplication but if your code will get more specific and different repositories will have to support specific queries for different entity types, you end up with custom implementations of IRepository which then you will have to resolve somehow from Unit of Work. When it happens you'll see either people casting IRepository to concrete type which smells more in my opinion or exposing getters for these concrete implementations, rendering Unit of Work inconsistent. – yoger Oct 15 '19 at 10:31
2

I would first determine if I am doing dependency injection or not. Your first option is using dependency injection properly, the second option is not using it correctly. Although, just because you are using a dependency does not mean you have to expose it outside your class. For example, with constructor injection you have in your implementation class, then there isn't any need (with the code you've shown) to expose the IUserRepository, etc.

If you wanted a generic repository, it is still possible while using proper dependency injection. Even Microsoft's DI framework supports it. In this case, the implementation is essentially the same:

public class SqlUnitOfWork : IUnitOfWork
{
    private readonly MyDbContext _dbContext;

    public SqlUnitOfWork(MyDbContext dbContext,
        IRepository<User> userRepository,
        IRepository<UserHistory> userHistoryRepository,
        IRepository<UserSecurity> userSecurityRepository)
    {
        _dbContext = dbContext;
        UserRepository = userRepository;
        UserHistoryRepository = userHistoryRepository;
        UserSecurityRepository = userSecurityRepository;
    }

    public IRepository<User> UserRepository { get; }

    public IRepository<UserHistory> UserHistoryRepository { get; }

    public IRepository<UserSecurity> UserSecurityRepository { get; }

    public void Commit()
    {
        _dbContext.SaveChanges();
    }
}

and your interface is tweaked just slightly:

public interface IUnitOfWork
{
    IRepository<User> UserRepository { get; }
    IRepository<UserSecurity> UserSecurityRepository { get; }
    IRepository<UserHistory> UserHistoryRepository { get; }
    void Commit()
}

The approach above lets you use the best of both worlds, and allows you to have a generic interface to ensure all repositories are treated equally.


A more generic criticism here is that UnitOfWork seems to be a bad name for an object with a bunch of repositories exposed. There is nothing there that suggests that this is a self contained unit of work. You could loosely call the interface we defined the IUserManager or something like that.

If you were truly trying to encapsulate a unit of work, then you would have a data object that represents the pending commands and changes. For example...

  • Update: User
  • Create: UserHistory

Once the unit of work makes its way to the WorkHandler, then it decides what calls it needs to make to which IRepository, checking permissions, etc. The work handler would have an interface like this:

public interface WorkManager
{
    void Process(UnitOfWork work);
}

The implementation would read your unit of work list of commands and commit them.

| improve this answer | |
  • 1
    (1) "UnitOfWork seems to be a bad name for an object with a bunch of repositories exposed" I respectfully disagree. As a simple example, EF's db context is often (correctly) called a unit of work in and of itself, and in reality its main purpose to consumers is to act as an object with a bunch of DbSet<T> properties. (2) "There is nothing there that suggests that this is a self contained unit of work." Purpose is revealed through naming. It is the name that reveals that this class behaves as a unit of work (since it's literally called that). – Flater Oct 16 '19 at 9:07
  • (3) "If you were truly trying to encapsulate a unit of work, then you would have a data object that represents the pending commands and changes" The unit of work has a DbContext (which all repositories presumably depend on), which inherently has a changetracker to provide exactly that. OP's unit of work is merely a wrapper around the DbContext to avoid leaking EF outside of the DAL. If the DAL's consumers need change tracking, the UOW can expose this (based on the EF changetracker). If the DAL's consumers don't need it, then it doesn't need to be exposed in the UOW. – Flater Oct 16 '19 at 9:13
  • @Flater, Perhaps I am too old school, or worked with too many languages to assume that the term Unit of Work was simply a container for database access. When I see that term, I think more in lines with the Command pattern than what you are describing. Another term that comes to mind is a Job. That is the genesis of my comments. We obviously have very different viewpoints on the matter, and are approaching this from very different perspectives. Let's simply agree to disagree. – Berin Loritsch Oct 16 '19 at 14:00
  • Just to add my $.02, to me a "unit of work" is not unlike what you would call a database "transaction". That is to say, it is a set of data operations that are committed in an "all or nothing" fashion. UOWs can exist without involving a database, but databases are by far the most common usage for UOWs. – Flater Oct 16 '19 at 14:03
1

Take the best from both worlds

I really like generic repositories. Especially when dealing with EF, basic CRUD actions are all the same and can easily be contained in a generic repository.

But the vast majority of time, when development continues beyond the basic in-and-out data management, you end up needing entity-specific operations which can't be contained by a generic repository. Often, I start with generic repositories and expand them as I go along, e.g.

public class UnitOfWork
{
    public GenericRepository<Foo> FooRepository { get; private set; }
    public GenericRepository<Bar> BarRepository { get; private set; }
    public GenericRepository<Baz> BazRepository { get; private set; }
}

Suppose I later realize I need a Foo-specific query, I'll create it on a FooRepository which derives from the generic repo:

public class FooRepository : GenericRepository<Foo>
{
    public IEnumerable<Foo> MySpecificQuery() {}
}

And then I update the unit of work:

public class UnitOfWork
{
    public FooRepository FooRepository { get; private set; }
    public GenericRepository<Bar> BarRepository { get; private set; }
    public GenericRepository<Baz> BazRepository { get; private set; }
}

This is in my opinion the better approach. It keeps things generic for as much as it can, but allows for entity-specific extensions (or overrides) where necessary.


Making life easier

However, I would suggest one improvement. Having taken this approach several times, I started getting annoyed at how often I needed to rewrite the UOW definition whenever a repository outgrew its generic nature.

Because of this, in codebases where I expect most (if not all) repositories to end up being extended, I immediately start with defining specific repository types, but I keep them empty.

public class FooRepository : GenericRepository<Foo> {}

public class BarRepository : GenericRepository<Bar> {}

public class BazRepository : GenericRepository<Baz> {}

public class UnitOfWork
{
    public FooRepository FooRepository { get; private set; }
    public BarRepository BarRepository { get; private set; }
    public BazRepository BazRepository { get; private set; }
}

It's possibly that one of these repo classes never gets filled in, but I prefer having it already available so I don't need to keep changing the UOW interface constantly.

Personally, I tend to autogenerate these files when I set up the initial domain model. I have a small console tool that generates copies of files in which it stringreplaces the entity names. I don't rely on generation in general, I just do it once because I hate having to repetitively do the same boring job 20 to 30 times.

It's just an initial setup tip to make life a bit nicer.


Generic factory method

Notice that I didn't use a generic repository factory method like you did. I used them in the past, but this doesn't work well with the approach I've outlined here. The problem is that if you extend FooRepository, you have to run down all usage of uow.GetRepository<Foo>() and replacing them with the property call.

I find that to be a cumbersome task, which is why I predefine all my repositories explicitly, at least as far as the public interface of my unit of work is concerned. Internally, you can still use the method if you prefer, e.g.:

public class UnitOfWork
{
    public GenericRepository<Foo> FooRepository => GetRepository<Foo>();
    public GenericRepository<Bar> BarRepository => GetRepository<Bar>();
    public GenericRepository<Baz> BazRepository => GetRepository<Baz>();

    protected GenericRepository<T> GetRepository<T>()
    {
        return new GenericRepository<T>(this.DbContext); //oversimplified example
    }
}

However, as if you take my improvement into account (the previous section of my answer), you can't take this route since you'd already be using the entity specific repository types and thus can't rely on a generic factory method.


Container

Your Container class is a service locator. This is one of those things that looks really neat and concise when you first start using it. I suspect that many developers will naturally come across this idea at some point and will usually get really excited about it.

From a technical perspective, it works well when it works. But it has a severe drawback: it obscures your dependency graph in a way that makes it really hard for consumers to know which dependencies they need to set up.

There are several online blog posts that go into depth on why service locators are an antipattern

In reality, the problem with service pattern is that it hides a class's dependencies and is a bonafide anti-pattern. In fact, it takes away a developer's clarity about the class he/she is using. While we have so many issues with the service locator, using constructor injection can save our lives.

| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.