Allowing query handlers to use other query handlers

I recently started looking into CQRS when using Entity Framework and the impact it has had on my systems has been overwhelming.

I implemented the following patterns described in these blog posts:

• Query implementation

• Command implementation

I have implemented these with Autofac as dependency injection, and Entity Framework with code first.

Now for the question. I have decided to let my query handlers use each other, and build upon other query handlers results. Is that considered okay? What issues may I run into in the future (if any)? Is the code SOLID?

Here's an example of my GetEmployeesQueryHandler:

class GetEmployeesQueryHandler : IQueryHandler>
{
    private readonly DatabaseContext database;

    public GetEmployeesQueryHandler(DatabaseContext database)
    {
        this.database = database;
    }

    public IQueryable Handle(GetEmployeesQuery query)
    {
        return database.Employees;
    }
}

My Entity Framework DatabaseContext gets injected into the constructor, and I simply return all employees.

Then I have a more specific query, which bases itself on this query. It does that by first fetching all employees, and then filtering them based on the Id of the employee.

class GetEmployeeByIdQueryHandler : IQueryHandler
{
    private readonly IQueryProcessor processor;

    public GetEmployeeByIdQueryHandler(IQueryProcessor container)
    {
        this.processor = container;
    }

    public Employee Handle(GetEmployeeByIdQuery query)
    {
        var employees = processor.Process(new GetEmployeesQuery());
        return employees.SingleOrDefault(e => e.Id == query.EmployeeId);
    }
}

And that's it. What do you think? Is it okay for my GetEmployeesByIdQueryHandler to use the GetEmployeesQueryHandler to get all employees first, for better code reuse?

Note that I return an IQueryable when I fetch all employees, so the SQL query will still be lazily generated, and evaluated in an optimized way.

For those who want

I have decided to let my query handlers use each other, and build upon other query handlers results. Is that considered okay? What issues may I run into in the future (if any)? Is the code SOLID?

You are letting components be built up by other components. This is completely SOLID and is a good approach.

What I would like to warn about is returning IQueryable. This is fine as long as the query handler is used by other query handlers (since this enables composible queries with good performance), don't return an IQueryable to the presentation layer. This makes the system unreliable and hard to test. This means that you will have to implement sorting and paging inside your business layer, but this is actually quite easy.

Note that your components should not dispose any dependencies that are injected into him. The reason is that such component doesn't own the dependency and has no idea what the lifetime of the dependency is. Disposing that dependency can make the application break, and this is something you already noted in the comments.

As a matter of fact, you are violating SOLID by injecting a dependency that implements IDisposable; you are violating the Dependency Inversion Principle. This principle states that "abstractions should not depend on details", but IDisposable is an implementation detail. If you prevent having IDisposable on the abstraction, but instead place it on the implementation, you'll see that it becomes impossible for the consumer to call Dispose() (which is good, because it has no idea whether or not it should call it), and now only the one who created that dependency can dispose that dependency (which is your composition root). This makes your application code much simpler, because you will hardly ever need to implement IDisposable at all.

In your case however, you can't remove IDisposable from the DatabaseContext, because you inherit from DbContext. But injecting a DbContext is itself a DIP violation. Although not all DIP violations are bad (and you will always have DIP violations somewhere in your application), I rather hide the DbContext from my code, for instance by using an IUnitOfWork abstraction with a single IQueryable Set() method. Advantage of this is that the DbContext can be resolved at runtime, instead of injected into consumers, and it allows to easily wrap the IUnitOfWork with some sort of security decorator that filters the results based on the user's rights.

Do note that the part of the system that created a disposable component typically holds the ownership, and is therefore responsible of disposing that. Although this ownership can be transferred, you should typically not do this, because this complicates your application (I think you already noticed this). So you composition root creates this dependency and should dispose it. In case you use a DI library, the library will create that insance for you. In that case, the library is also responsible of disposing it for you. Although you can view this as 'something magical', IMO it's simply a basic feature of the library you are using. You should understand the libraries you are use. In the case of Autofac, you can be pretty sure that Autofac handles this correctly for you. In case you would switch back to Pure DI (formally known as poor man's DI), your code will obviously again be in control over that dependency, and you will have to implement this disposal again manually. There's no design smell here. But that said, although disposing is not the problem, scoping might actually be. Please read this answer of mine.

Although the query handler pattern might seem over-engineered at first, if you read the article closely, you'll see that it is simply an implementation of the SOLID principles. IMO, you should always strive to adhere to the SOLID principles in the core parts of your application. Querying is obviously a core part of every application. My experience is that query handlers even work well on small projects. They allow adding cross-cutting concerns with such easy, that it can really boost the productivity and flexibility in smaller applications.

Using IQueryProcessor instead of injecting IQueryHandlers directly has some downsides, such as the fact that handlers get resolved lazily at runtime. This makes it harder to verify the complete object graph and it makes it hard to see what queries a component executes. On the other hand, it makes your code cleaner, because the generic types and often long names for query classes can give some noise in your code (that's more a limitation in C# than a limitation of the pattern). In that case the IQueryProcessor can help.

Since your IQueryProcessor implementation will be part of your composition root, it is completely fine to have a dependency on the container, since the rest of your composition will have a dependency on the container as well (although it would be good to call it AutofacQueryProcessor). It is incorrect to assum

StackExchange Code Review Q#87876, answer score: 9