Should I decouple the repository interface from the domain model

Question

Let’s say I have some DDD service that requires some IEnumerable<Foo> to perform some calculations. I came up with two designs:

1. Abstract the data access with an IFooRepository interface, which is quite typical

   public class FooService
   {
       private readonly IFooRepository _fooRepository;
   
       public FooService(IFooRepository fooRepository)
           => _fooRepository = fooRepository;
   
   
       public int Calculate()
       {
           var fooModels = _fooRepository.GetAll();
           return fooModels.Sum(f => f.Bar);
       }
   }
   

2. Do not rely on the IFooRepository abstraction and inject IEnumerable<Foo> directly

   public class FooService
   {
       private readonly IEnumerable<Foo> _foos;
   
       public FooService(IEnumerable<Foo> foos)
           => _foos = foos;
   
   
       public int Calculate()
           => _foos.Sum(f => f.Bar);
   }    
   

This second design seems better in my opinion as FooService now does not care where the data is coming from and Calculate becomes pure domain logic (ignoring the fact that IEnumerable may come from an impure source).

Another argument for using the second design is that when IFooRepository performs asynchronous IO over the network, usually it will be desirable to use async-await like:

public class AsyncDbFooRepository : IFooRepository
{
    public async Task<IEnumerable<Foo>> GetAll()
    {
        // Asynchronously fetch results from database
    }
}

But as you need to async all the way down, FooService is now forced to change its signature to async Task<int> Calculate(). This seems to violate the dependency inversion principle.

However, there are also issues with the second design. First of all, you have to rely on the DI container (using Simple Injector as an example here) or the composition root to resolve the data access code like:

public class CompositionRoot
{
    public void ComposeDependencies()
    {
        container.Register<IFooRepository, AsyncDbFooRepository>(Lifestyle.Scoped);

        // Not sure if the syntax is right, but it demonstrates the concept
        container.Register<FooService>(async () => new FooService(await GetFoos(container)));
    }

    private async Task<IEnumerable<Foo>> GetFoos(Container container)
    {
        var fooRepository = container.GetInstance<IFooRepository>();
        return await fooRepository.GetAll();
    }
}

Also in my specific scenario, AsyncDbFooRepository requires some sort of runtime parameter to construct, and that means you need an abstract factory to construct AsyncDbFooRepository.

With the abstract factory, now I have to manage the life cycles of all dependencies under AsyncDbFooRepository (the object graph under AsyncDbFooRepository is not trivial). I have a hunch that I am using DI incorrectly if I opt for the second design.

---

In summary, my questions are:

1. Am I using DI incorrectly in my second design?
2. How can I compose my dependencies satisfactorily for my second design?

Answer 1

One aspect of async/await is that it by definition needs to applied "all the way down" as you rightfully state. You however can't prevent the use of Task<T> when injecting an IEnumerable<T>, as you suggest in your second option. You will have to inject a Task<IEnumerable<T>> into constructors to ensure data is retrieved asynchronously. When injecting an IEnumerable<T> it either means that your thread gets blocked when the collection is enumerated -or- all data must be loaded during object graph construction.

Loading data during object graph construction however is problematic, because of the reasons I explained here. Besides that, since we're dealing with collections of data here, it means that all data must be fetched from the database on each request, even though not all data might be required or even used. This might cause quite a performance penalty.

Am I using DI incorrectly in my second design?

That's hard to say. An IEnumerable<T> is a stream, so you could consider it a factory, which means that injecting an IEnumerable<T> does not require the runtime data to be loaded during object construction. As long as that condition is met, injecting an IEnumerable<T> could be fine, but still makes it impossible to make the system asynchronous.

However, when injecting an IEnumerable<T> you might end up with ambiguity, because it might not be very clear what it means to be injecting an IEnumerable<T>. Is that collection a stream that is lazily evaluated or not? Does it contain all elements of T. Is T runtime data or a service?

To prevent this confusion, moving the loading of this runtime information behind an abstraction is typically the best thing to do. To make your life easier, you could make the repository abstraction generic as well:

public interface IRepository<T> where T : Entity
{
    Task<IEnumerable<T>> GetAll();
}

This allows you to have one generic implementation and make one single registration for all entities in the system.

How can I compose my dependencies satisfactorily for my second design?

You can't. To be able to do this, your DI container must be able to resolve object graphs asynchronously. For instance, it requires the following API:

Task<T> GetInstanceAsync<T>()

But Simple Injection doesn't have such API, and neither does any other existing DI Container and that's for good reason. The reason is that object construction must be simple, fast and reliable and you lose that when doing I/O during object graph construction.

So not only is your second design undesirable, it is impossible to do so when data is loaded during object construction, without breaking the asynchonicity of the system and causing threads to block while using a DI container.

Answer 2

I try as much as possible (until now I've succeded every time) to not inject any service that do IO in my domain models as I like to keep them pure with no side effects.

That being said the second solution seems better but there is a problem with the signature of the method public int Calculate(): it uses some hidden data to perform the calculation, so it is not explicit. In cases like this I like to pass the transient input data as input parameter directly to the method like this:

public int Calculate(IEnumerable<Foo> foos)

In this way it is very clear what the method needs and what it returns (based on the combination of class name and method name).