james mckay dot net
because there are few things that are less logical than business logic

Posts tagged: design

Query Objects: a better approach than your BLL/repository

If you’ve been following what I’ve been saying here on my blog and on the ASP.NET forums over the past month or so, you’ll no doubt realise that I’m not a fan of the traditional layered architecture, with your presentation layer only allowed to talk to your business layer, your business layer only allowed to talk to your repository, only your repository allowed to talk to your ORM, and all of them in separate assemblies for no reason whatsoever other than That Is How You Are Supposed To Do It. It adds a lot of friction and ceremony, it restricts you in ways that are harmful, its only benefits are unnecessary and dubious, and every implementation of it that I’ve come across has been horrible.

Here’s a far better approach:

public class BlogController : Controller
{
    private IBlogContext _context;

    public BlogController(IBlogContext context)
    {
        _context = context;
    }

    public ActionResult ShowPosts(PostsQuery query)
    {
        query.PrefetchComments = false;
        var posts = query.GetPosts(_context);
        return View(posts);
    }
}

[Bind(Exclude="PrefetchComments")]
public class PostsQuery
{
    private const int DefaultPageSize = 10;

    public int? PageNumber { get; set; }
    public int? PageSize { get; set; }
    public bool Descending { get; set; }
    public bool PrefetchComments { get; set; }

    public IQueryable<Post> GetPosts(IBlogContext context)
    {
        var posts = Descending
            ? context.Posts.OrderByDescending
                (post => post.PostDate)
            : context.Posts.OrderBy(post => post.PostDate);
        if (PrefetchComments) {
            posts = posts.Include("Comments");
        }
        if (PageNumber.HasValue && PageNumber > 1) {
            posts = posts.Skip
                ((PageNumber - 1) * (PageSize ?? DefaultPageSize));
        }
        posts = posts.Take(PageSize ?? DefaultPageSize);
        return posts;
    }
}

A few points to note here.

First, you are injecting your Entity Framework DbContext subclass (the implementation of IBlogContext) directly into your controllers. Get over it: it’s not as harmful as you think it is. Your IOC container can (and should) manage its lifecycle.

Secondly, your query object follows the Open/Closed Principle: you can easily add new sorting and filtering options without having to modify either the method signatures of your controllers or its own other properties and methods. With a query method on your Repository, on the other hand, adding new options would be a breaking change.

Thirdly, it is very easy to avoid SELECT n+1 problems on the one hand while at the same time not fetching screeds of data that you don’t need on the other, as the PrefetchComments property illustrates.

Fourthly, this approach is no less testable than your traditional BLL/BOL/DAL approach. By mocking your IBlogContext and IDbSet<T> interfaces, you can test your query object in isolation from your database. You would need to hit the database for more advanced Entity Framework features of course, but the same would be true with query methods on your repository.

Fifthly, note that your query object is automatically created and populated with the correct settings by ASP.NET MVC’s model binder.

All in all, a very simple, elegant and DRY approach.

(Hat tip: Jimmy Bogard for the original inspiration. This version simply adds the twist of having your query objects created and initialised by ASP.NET MVC’s model binder.)

Interchangeable data access layers == stealing from your client

This is one of those so-called “best practices” that crops up a lot in the .net world:

You need to keep the different layers of your application loosely coupled with a clean separation of concerns so that you can swap out your data access layer for a different technology if you need to.

It all sounds right, doesn’t it? Separation of concerns, loose coupling…very clean, SOLID, and Uncle Bob-compliant, right?

Just. A. Minute.

The separation of concerns you are proposing is high-maintenance, high-friction, usually unnecessary, obstructive to important performance optimisations and other requirements, and, as this post by Oren Eini aka Ayende Rahien points out, usually doesn’t work anyway.

In what universe is it a best practice to allocate development time and resources, for which your client is paying, towards implementing a high-maintenance, high-friction, broken, unnecessary, non-functional requirement that they are not asking for, at the expense of business value that they are?

In the universe where I live, that is called “stealing from your client.”

Nobody is saying here that separation of concerns is bad per se. What is bad, however, is inappropriate separation of concerns — an attempt to decouple parts of your system that don’t lend themselves to being decoupled. Kent Beck has a pretty good guideline as to when separation of concerns is appropriate and when it isn’t: you should be dealing with two parts of your system which you can reason about independently.

You can not reason about your business layer, your presentation layer, and your data access layer independently. User stories that require related changes right across all your layers are very, very common.

Every project that I’ve ever seen that has attempted this kind of abstraction has been riddled with severe SELECT n+1 problems that could not be resolved without breaking encapsulation.

(Nitpickers’ corner: I’m not talking about test mocks here. That’s different. It’s relatively easy to make your test mocks behave like Entity Framework. It’s orders of magnitude harder to make NHibernate or RavenDB behave like Entity Framework.)

If you can present a valid business case for making your persistence mechanism interchangeable, then it’s a different matter, of course. But in that case, you need to implement both (or all) the different options up-front right from the start, and to bear in mind that the necessary separation of concerns almost certainly won’t cleanly follow the boundary between your business layer and your DAL. You should also warn your client of the extra costs involved, otherwise you won’t be delivering good value for money.

The Anaemic Business Layer

The three-layer architecture, with your presentation layer, your business layer and your data access layer, is a staple of traditional .net applications, being heavily promoted on sites such as MSDN, CodeProject and the ASP.NET forums. Its advantage is that it is a fairly canonical way of doing things, so (in theory at least) when you get a new developer on the team, they should have no trouble in finding where everything is.

Its disadvantage is that it tends to breed certain antipatterns that crop up over and over and over again. One such antipattern is what I call the Anaemic Business Layer.

The Anaemic Business Layer is a close cousin of the Anaemic Domain Model, and often appears hand in hand with it. It is characterised by business “logic” classes that don’t actually have any logic in them at all, but only shunt data between the domain model returned from your ORM and a set of identical model classes with identical method signatures in a different namespace. Sometimes it may wrap all the calls to your repository in catch-log-throw blocks, which is another antipattern in itself, but that’s a rant for another time.

The problem with the Anaemic Business Layer is that it makes your code much more time consuming and difficult to maintain, since you have to drill down through more classes just to figure out what is going on, and you have to edit more files to make a single change. This in turn increases risk because it’s all too easy to overlook one of the places where you have to make a change. It also makes things restrictive, because you lose access to certain advanced features of your ORM such as lazy loading, query shaping, transaction management, cross-cutting concerns or concurrency control, that can only properly be handled in the business layer.

The Anaemic Business Layer is usually symptomatic of an over-strict and inflexible insistence on a “proper” layered architecture, where your UI is only allowed to talk to your business layer and your business layer is only allowed to talk to your data access layer. You could make an argument for the need for encapsulation — so that you can easily change the implementation of the methods in the business layer if need be — but that’s only really important if you’re producing an API for public consumption by the rest of the world. Your app is not the rest of the world, and besides, those specific changes tend not to happen (especially for basic CRUD operations), so I’d be inclined to call YAGNI on that one.

The other reason why you might have an Anaemic Business Layer is that you’ve got too much going on in your controllers or your data access layer. You shouldn’t have any business logic in either, as that hinders testability, especially if you’re of the school of thought that says your unit tests shouldn’t hit the database. But if that’s not the case, then it’s time to stop being so pedantic. An Anaemic Business Layer serves no purpose other than to get in the way and slow you down. So ditch your unhelpful faux-“best practices,” bypass it altogether, and go straight from your UI to your repository.