Asynchronous Injection by Mark Seemann
How to combine asynchronous programming with Dependency Injection without leaky abstractions.
C# has decent support for asynchronous programming, but it ultimately leads to leaky abstractions. This is often conspicuous when combined with Dependency Injection (DI). This leads to frequently asked questions around the combination of DI and asynchronous programming. This article outlines the problem and suggests an alternative.
The code base supporting this article is available on GitHub.
A synchronous example #
In this article, you'll see various stages of a small sample code base that pretends to implement the server-side behaviour of an on-line restaurant reservation system (my favourite example scenario). In the first stage, the code uses DI, but no asynchronous I/O.
At the boundary of the application, a Post method receives a Reservation object:
public class ReservationsController : ControllerBase { public ReservationsController(IMaîtreD maîtreD) { MaîtreD = maîtreD; } public IMaîtreD MaîtreD { get; } public IActionResult Post(Reservation reservation) { int? id = MaîtreD.TryAccept(reservation); if (id == null) return InternalServerError("Table unavailable"); return Ok(id.Value); } }
The Reservation object is just a simple bundle of properties:
public class Reservation { public DateTimeOffset Date { get; set; } public string Email { get; set; } public string Name { get; set; } public int Quantity { get; set; } public bool IsAccepted { get; set; } }
In a production code base, I'd favour a separation of DTOs and domain objects with proper encapsulation, but in order to keep the code example simple, here the two roles are combined.
The Post method simply delegates most work to an injected IMaîtreD object, and translates the return value to an HTTP response.
The code example is overly simplistic, to the point where you may wonder what is the point of DI, since it seems that the Post method doesn't perform any work itself. A slightly more realistic example includes some input validation and mapping between layers.
The IMaîtreD implementation is this:
public class MaîtreD : IMaîtreD { public MaîtreD(int capacity, IReservationsRepository repository) { Capacity = capacity; Repository = repository; } public int Capacity { get; } public IReservationsRepository Repository { get; } public int? TryAccept(Reservation reservation) { var reservations = Repository.ReadReservations(reservation.Date); int reservedSeats = reservations.Sum(r => r.Quantity); if (Capacity < reservedSeats + reservation.Quantity) return null; reservation.IsAccepted = true; return Repository.Create(reservation); } }
The protocol for the TryAccept method is that it returns the reservation ID if it accepts the reservation. If the restaurant has too little remaining Capacity for the requested date, it instead returns null. Regular readers of this blog will know that I'm no fan of null, but this keeps the example realistic. I'm also no fan of state mutation, but the example does that as well, by setting IsAccepted to true.
Introducing asynchrony #
The above example is entirely synchronous, but perhaps you wish to introduce some asynchrony. For example, the IReservationsRepository implies synchrony:
public interface IReservationsRepository { Reservation[] ReadReservations(DateTimeOffset date); int Create(Reservation reservation); }
In reality, though, you know that the implementation of this interface queries and writes to a relational database. Perhaps making this communication asynchronous could improve application performance. It's worth a try, at least.
How do you make something asynchronous in C#? You change the return type of the methods in question. Therefore, you have to change the IReservationsRepository interface:
public interface IReservationsRepository { Task<Reservation[]> ReadReservations(DateTimeOffset date); Task<int> Create(Reservation reservation); }
The Repository methods now return Tasks. This is the first leaky abstraction. From the Dependency Inversion Principle it follows that
The"clients [...] own the abstract interfaces"
MaîtreD class is the client of the IReservationsRepository interface, which should be designed to support the needs of that class. MaîtreD doesn't need IReservationsRepository to be asynchronous.
The change of the interface has nothing to with what MaîtreD needs, but rather with a particular implementation of the IReservationsRepository interface. Because this implementation queries and writes to a relational database, this implementation detail leaks into the interface definition. It is, therefore, a leaky abstraction.
On a more practical level, accommodating the change is easily done. Just add async and await keywords in appropriate places:
public async Task<int?> TryAccept(Reservation reservation) { var reservations = await Repository.ReadReservations(reservation.Date); int reservedSeats = reservations.Sum(r => r.Quantity); if (Capacity < reservedSeats + reservation.Quantity) return null; reservation.IsAccepted = true; return await Repository.Create(reservation); }
In order to compile, however, you also have to fix the IMaîtreD interface:
public interface IMaîtreD { Task<int?> TryAccept(Reservation reservation); }
This is the second leaky abstraction, and it's worse than the first. Perhaps you could successfully argue that it was conceptually acceptable to model IReservationsRepository as asynchronous. After all, a Repository conceptually represents a data store, and these are generally out-of-process resources that require I/O.
The IMaîtreD interface, on the other hand, is a domain object. It models how business is done, not how data should be accessed. Why should business logic be asynchronous?
It's hardly news that async and await is infectious. Once you introduce Tasks, it's async all the way!
That doesn't mean that asynchrony isn't one big leaky abstraction. It is.
You've probably already realised what this means in the context of the little example. You must also patch the Post method:
public async Task<IActionResult> Post(Reservation reservation) { int? id = await MaîtreD.TryAccept(reservation); if (id == null) return InternalServerError("Table unavailable"); return Ok(id.Value); }
Pragmatically, I'd be ready to accept the argument that this isn't a big deal. After all, you just replace all return values with Tasks, and add async and await keywords where they need to go. This hardly impacts the maintainability of a code base.
In C#, I'd be inclined to just acknowledge that, hey, there's a leaky abstraction. Moving on...
On the other hand, sometimes people imply that it has to be like this. That there is no other way.
Falsifiable claims like that often get my attention. Oh, really?!
Move impure interactions to the boundary of the system #
We can pretend that Task<T> forms a functor. It's also a monad. Monads are those incredibly useful programming abstractions that have been propagating from their origin in statically typed functional programming languages to more mainstream languages like C#.
In functional programming, impure interactions happen at the boundary of the system. Taking inspiration from functional programming, you can move the impure interactions to the boundary of the system.
In the interest of keeping the example simple, I'll only move the impure operations one level out: from MaîtreD to ReservationsController. The approach can be generalised, although you may have to look into how to handle pure interactions.
Where are the impure interactions in MaîtreD? They are in the two interactions with IReservationsRepository. The ReadReservations method is non-deterministic, because the same input value can return different results, depending on the state of the database when you call it. The Create method causes a side effect to happen, because it creates a row in the database. This is one way in which the state of the database could change, which makes ReadReservations non-deterministic. Additionally, Create also violates Command Query Separation (CQS) by returning the ID of the row it creates. This, again, is non-deterministic, because the same input value will produce a new return value every time the method is called. (Incidentally, you should design Create methods so that they don't violate CQS.)
Move reservations to a method argument #
The first refactoring is the easiest. Move the ReadReservations method call to the application boundary. In the above state of the code, the TryAccept method unconditionally calls Repository.ReadReservations to populate the reservations variable. Instead of doing this from within TryAccept, just pass reservations as a method argument:
public async Task<int?> TryAccept( Reservation[] reservations, Reservation reservation) { int reservedSeats = reservations.Sum(r => r.Quantity); if (Capacity < reservedSeats + reservation.Quantity) return null; reservation.IsAccepted = true; return await Repository.Create(reservation); }
This no longer compiles until you also change the IMaîtreD interface:
public interface IMaîtreD { Task<int?> TryAccept(Reservation[] reservations, Reservation reservation); }
You probably think that this is a much worse leaky abstraction than returning a Task. I'd be inclined to agree, but trust me: ultimately, this will matter not at all.
When you move an impure operation outwards, it means that when you remove it from one place, you must add it to another. In this case, you'll have to query the Repository from the ReservationsController, which also means that you need to add the Repository as a dependency there:
public class ReservationsController : ControllerBase { public ReservationsController( IMaîtreD maîtreD, IReservationsRepository repository) { MaîtreD = maîtreD; Repository = repository; } public IMaîtreD MaîtreD { get; } public IReservationsRepository Repository { get; } public async Task<IActionResult> Post(Reservation reservation) { var reservations = await Repository.ReadReservations(reservation.Date); int? id = await MaîtreD.TryAccept(reservations, reservation); if (id == null) return InternalServerError("Table unavailable"); return Ok(id.Value); } }
This is a refactoring in the true sense of the word. It just reorganises the code without changing the overall behaviour of the system. Now the Post method has to query the Repository before it can delegate the business decision to MaîtreD.
Separate decision from effect #
As far as I can tell, the main reason to use DI is because some impure interactions are conditional. This is also the case for the TryAccept method. Only if there's sufficient remaining capacity does it call Repository.Create. If it detects that there's too little remaining capacity, it immediately returns null and doesn't call Repository.Create.
In object-oriented code, DI is the most common way to decouple decisions from effects. Imperative code reaches a decision and calls a method on an object based on that decision. The effect of calling the method can vary because of polymorphism.
In functional programming, you typically use a functor like Maybe or Either to separate decisions from effects. You can do the same here.
The protocol of the TryAccept method already communicates the decision reached by the method. An int value is the reservation ID; this implies that the reservation was accepted. On the other hand, null indicates that the reservation was declined.
You can use the same sort of protocol, but instead of returning a Nullable<int>, you can return a Maybe<Reservation>:
public async Task<Maybe<Reservation>> TryAccept( Reservation[] reservations, Reservation reservation) { int reservedSeats = reservations.Sum(r => r.Quantity); if (Capacity < reservedSeats + reservation.Quantity) return Maybe.Empty<Reservation>(); reservation.IsAccepted = true; return reservation.ToMaybe(); }
This completely decouples the decision from the effect. By returning Maybe<Reservation>, the TryAccept method communicates the decision it made, while leaving further processing entirely up to the caller.
In this case, the caller is the Post method, which can now compose the result of invoking TryAccept with Repository.Create:
public async Task<IActionResult> Post(Reservation reservation) { var reservations = await Repository.ReadReservations(reservation.Date); Maybe<Reservation> m = await MaîtreD.TryAccept(reservations, reservation); return await m .Select(async r => await Repository.Create(r)) .Match( nothing: Task.FromResult(InternalServerError("Table unavailable")), just: async id => Ok(await id)); }
Notice that the Post method never attempts to extract 'the value' from m. Instead, it injects the desired behaviour (Repository.Create) into the monad. The result of calling Select with an asynchronous lambda expression like that is a Maybe<Task<int>>, which is a awkward combination. You can fix that later.
The Match method is the catamorphism for Maybe. It looks exactly like the Match method on the Church-encoded Maybe. It handles both the case when m is empty, and the case when m is populated. In both cases, it returns a Task<IActionResult>.
Synchronous domain logic #
At this point, you have a compiler warning in your code:
Warning CS1998 This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.Indeed, the current incarnation of
TryAccept is synchronous, so remove the async keyword and change the return type:
public Maybe<Reservation> TryAccept( Reservation[] reservations, Reservation reservation) { int reservedSeats = reservations.Sum(r => r.Quantity); if (Capacity < reservedSeats + reservation.Quantity) return Maybe.Empty<Reservation>(); reservation.IsAccepted = true; return reservation.ToMaybe(); }
This requires a minimal change to the Post method: it no longer has to await TryAccept:
public async Task<IActionResult> Post(Reservation reservation) { var reservations = await Repository.ReadReservations(reservation.Date); Maybe<Reservation> m = MaîtreD.TryAccept(reservations, reservation); return await m .Select(async r => await Repository.Create(r)) .Match( nothing: Task.FromResult(InternalServerError("Table unavailable")), just: async id => Ok(await id)); }
Apart from that, this version of Post is the same as the one above.
Notice that at this point, the domain logic (TryAccept) is no longer asynchronous. The leaky abstraction is gone.
Redundant abstraction #
The overall work is done, but there's some tidying up remaining. If you review the TryAccept method, you'll notice that it no longer uses the injected Repository. You might as well simplify the class by removing the dependency:
public class MaîtreD : IMaîtreD { public MaîtreD(int capacity) { Capacity = capacity; } public int Capacity { get; } public Maybe<Reservation> TryAccept( Reservation[] reservations, Reservation reservation) { int reservedSeats = reservations.Sum(r => r.Quantity); if (Capacity < reservedSeats + reservation.Quantity) return Maybe.Empty<Reservation>(); reservation.IsAccepted = true; return reservation.ToMaybe(); } }
The TryAccept method is now deterministic. The same input will always return the same input. This is not yet a pure function, because it still has a single side effect: it mutates the state of reservation by setting IsAccepted to true. You could, however, without too much trouble refactor Reservation to an immutable Value Object.
This would enable you to write the last part of the TryAccept method like this:
return reservation.Accept().ToMaybe();
In any case, the method is close enough to be pure that it's testable. The interactions of TryAccept and any client code (including unit tests) is completely controllable and observable by the client.
This means that there's no reason to Stub it out. You might as well just use the function directly in the Post method:
public class ReservationsController : ControllerBase { public ReservationsController( int capacity, IReservationsRepository repository) { Capacity = capacity; Repository = repository; } public int Capacity { get; } public IReservationsRepository Repository { get; } public async Task<IActionResult> Post(Reservation reservation) { var reservations = await Repository.ReadReservations(reservation.Date); Maybe<Reservation> m = new MaîtreD(Capacity).TryAccept(reservations, reservation); return await m .Select(async r => await Repository.Create(r)) .Match( nothing: Task.FromResult(InternalServerError("Table unavailable")), just: async id => Ok(await id)); } }
Notice that ReservationsController no longer has an IMaîtreD dependency.
All this time, whenever you make a change to the TryAccept method signature, you'd also have to fix the IMaîtreD interface to make the code compile. If you worried that all of these changes were leaky abstractions, you'll be happy to learn that in the end, it doesn't even matter. No code uses that interface, so you can delete it.
Grooming #
The MaîtreD class looks fine, but the Post method could use some grooming. I'm not going to tire you with all the small refactoring steps. You can follow them in the GitHub repository if you're interested. Eventually, you could arrive at an implementation like this:
public class ReservationsController : ControllerBase { public ReservationsController( int capacity, IReservationsRepository repository) { Capacity = capacity; Repository = repository; maîtreD = new MaîtreD(capacity); } public int Capacity { get; } public IReservationsRepository Repository { get; } private readonly MaîtreD maîtreD; public async Task<IActionResult> Post(Reservation reservation) { return await Repository.ReadReservations(reservation.Date) .Select(rs => maîtreD.TryAccept(rs, reservation)) .SelectMany(m => m.Traverse(Repository.Create)) .Match(InternalServerError("Table unavailable"), Ok); } }
Now the Post method is just a single, composed asynchronous pipeline. Is it a coincidence that this is possible?
This is no coincidence. This top-level method executes in the 'Task monad', and a monad is, by definition, composable. You can chain operations together, and they don't all have to be asynchronous. Specifically, maîtreD.TryAccept is a synchronous piece of business logic. It's unaware that it's being injected into an asynchronous context. This type of design would be completely run of the mill in F# with its asynchronous workflows.
Summary #
Dependency Injection frequently involves I/O-bound operations. Those typically get hidden behind interfaces so that they can be mocked or stubbed. You may want to access those I/O-bound resources asynchronously, but with C#'s support for asynchronous programming, you'll have to make your abstractions asynchronous.
When you make the leaf nodes in your call graph asynchronous, that design change ripples through the entire code base, forcing you to be async all the way. One result of this is that the domain model must also accommodate asynchrony, although this is rarely required by the logic it implements. These concessions to asynchrony are leaky abstractions.
Pragmatically, it's hardly a big problem. You can use the async and await keywords to deal with the asynchrony, and it's unlikely to, in itself, cause a problem with maintenance.
In functional programming, monads can address asynchrony without introducing sweeping leaky abstractions. Instead of making DI asynchronous, you can inject desired behaviour into an asynchronous context.
Behaviour Injection, not Dependency Injection.
Comments
Hi Mark,
aren't you loading more responsibilities on the
ReservationsController? Previously, it only had to delegate all the work toMaîtreDand return an appropriate result, now it additionally fetches reservations from the repository. You are also loading the handling of any errors the reservations repository might throw onto the controller, instead of handling them in theMaîtreDclass.You are also hard wiring a dependency on
MaîtreDinto theReservationsController; I thought one of the advantages of DI were to avoid newing up dependencies to concrete implementations outside of a centralized "builder class".Could you elaborate on these points? Thanks!
Ramon, thank you for writing. Am I loading more responsibilities on the Controller? Yes, I am. Too many? I don't think so.
To be fair, however, this example is unrealistically simplified (in order to make it easily understandable). There isn't much going on, overall, so one has to imagine that more things are happening than is actually the case. For instance, at the beginning of the example, so little is going on in the Controller that I think it'd be fair to ask why it's even necessary to distinguish between a Controller and a
MaîtreDclass.Usually, I'd say that the responsibility of a Controller object is to facilitate the translation of what goes on at the boundary of the application and what happens in the domain model. Using the terminology of the ports and adapters architecture, you could say that a Controller's responsibility is to serve as an Adapter between the technology-agnostic domain model and the technology-specific SDKs you'll need to bring into play to communicate with the 'real world'. Talking to databases fits that responsibility, I think.
The
MaîtreDclass didn't handle any database errors before, so I don't agree that I've moved that responsibility.When it comes to using a
MaîtreDobject from inside the Controller, I don't agree that I've 'hard-wired' it. It's not a dependency in the Dependency Injection sense; it's an implementation detail. Notice that it's aprivateclass field.Is it an 'advantage of DI' that you can "avoid newing up dependencies to concrete implementations outside of a centralized "builder class"?" How is that an advantage? Is that a goal?
In future articles, I'll discuss this sort of 'dependency elimination' in more details.
Mark, thanks for replying.
I assumed that some exception handling would be happening in the
MaitreDclass that would then migrate to theReservationsControllerand you left it out for the sake of simplicity. But granted, that can still happen inside the respository class.Let's imagine that for some reason, you want to write to the filesystem in addition to the database (eg. writing some reservation data like table number that can be printed and given to the customer). Following your reasoning, there would now be a reference to some
IReservationPrinterin the Controller. It suddenly has to hold references to all data exchange classes that it was previously unaware of, only caring about the resultMaîtreDwas returning.Maybe I didn't express myself properly: I thought Dependency Injection is a technique to resolve all implementation types at a single composition root. Of course this only applies to dependencies in the sense of DI, so where do you draw the line between implementation detail and dependency?
In any case I'm looking forward to reading more articles on this topic!
Ramon, in general when it comes to exception handling, you either handle exceptions at the source (i.e. in the Repository) or at the boundary of the application (which is typically done by frameworks already). I'm no fan of defensive coding.
Yes, but nowMaîtreDdoesn't have to do that. Is there anything inherently associated with business logic that stipulates that it handles data access?The following line of argument may be increasingly difficult to relate to as time moves forward, and business becomes increasingly digital, but there once was a time when business logic was paper-based. In paper-based organisations, data would flow through a business in the shape of paper; typically as forms. Data would arrive at the desk of a clerk or domain expert who would add more data or annotations to a form, and put it in his or her out-box for later collection.
My point is that I see nothing inherent in business logic to stipulate that business objects should be responsible for data retrieval or persistence. I recommend Domain Modeling Made Functional if you're interested in a comprehensive treatment of this way of looking at modelling business logic.
It is, and that still happens here. There are, however, fewer dependencies overall. I would argue that with the final design outlined here, the remaining dependency (IReservationsRepository) is also, architecturally, the only real dependency of the application. The initialIMaîtreDdependency is, in my opinion, an implementation detail. Exposing it as a dependency makes the code more brittle, and harder to refactor, but that's what I'm going to cover in future articles.Mark, I have to admit that I'm still not convinced (without having read the book you mentioned):
Expanding on your analogy, a clerk would maybe make a phone call or walk over to another desk if he needs more information regarding his current form (I know I do at my office). A maître d'hôtel would presumably open his book of reservations to check if he still has a table available and would write a new reservation in his book.
The
MaîtreDdoesn't need to know if the data it needs comes from the file system or a database or a web service (that's the responsibility of the repository class), all it cares about is that it needs some data. Currently, some other part of the system decides what dataMaîtreDhas to work with.Again, I didn't have a look at the reading recommendation yet. Maybe I should. ;)
I definitely agree with Mark that the business logic (in the final version of
MaîtreD.TryAccept) should be in a function that is pure and synchronous. However, I am also sympathetic to Ramon's argument.There are two UIs for the application that I am currently building at work. The primary interface is over HTTP and uses web controllers just like in Mark's example. The second interface is a CLI (that is only accessable to administrators with phsyical access to the server). Suppose my application was also an on-line restaurant reservation system and that a reservation could be made with both UIs.
Looking back at the final implementation of
ReservationsController.Post, the first three lines are independent ofControllerBaseand would also need to be executed when accessing the system though the CLI. My understanding is that Ramon's primary suggestion is to move these three lines intoMaîtreD.TryAccept. I am sympathetic to Ramon's argument in that I am in favor of extracting those three lines. However, I don't want them to be colocated with the final implimentatiion ofMaîtreD.TryAccept.In my mind, the single responsibility of
ReservationsController.Postis to translate the result of the reseravation request into the expected type of response. That would be just the fourth line in the final implementation of this method. In terms of naming, I like Ramon's suggestion that the first three lines ofReservationsController.Postbe moved toMaîtreD.TryAccept. But then I also want to move the final implementation ofMaîtreD.TryAcceptto a method on a different type. As we all know, naming is an impossible problem, so I don't have a good name for this new third type.What do you think Ramon? Have I understood your concerns and suggested something that you could get behind?
What about you Mark? You said that there was
Would two UIs be sufficient motivation in your eyes to justify distinguishing between a Controller and aMaîtreDclass?Tyson, thank you for joining the discussion. By adding a particular problem (more than one user interface) to be addressed, you make the discussion more specific. I think this helps to clarify some issues.
Ramon wrote:
That's okay; you don't have to be. I rarely write articles with the explicit intent of telling people that they must do something, or that they should never do something else. While it does happen, this article isn't such an article. If it helps you address a problem, then take what you find useful. If it doesn't, then ignore it.With Tyson's help, though, we can now discuss something more concrete. I think some of those observations identify a tender spot in my line of argument. In the initial version of
ReservationsController, the only responsibility of thePostmethod was to translate from and to HTTP. That's a distinct separation of responsibility, so clearly preferable.When I add the
Repositorydependency, I widen the scope of theReservationsController's responsibility, which now includes 'all IO'. This does blur the demarcation of responsibility, but often still works out well in practice, I find. Still, it depends on how much other stuff is going on related to IO. If you have too much IO going on, another separation of responsibilities is in order.I do find, however, that when implementing the same sort of software capability in different user interfaces, I need to specifically design for each user interface paradigm. A web-based user interface is quite different from a command-line interface, which is again different from a native application, or a voice-based interface, and so on. A web-based interface is, for example, stateless, whereas a native smart phone application would often be stateful. You can rarely reuse the 'user interface controller layer' for one type of application in a different type of application.
Even a command-line interface could be stateful by interactively asking a series of questions. That's such a different user interface paradigm that an object designed for one type of interaction is rarely reusable in another context.
What I do find is that fine-grained building blocks still compose. When
Indeed, but how do you model this in software? A program doesn't have the degree of ad-hoc flexibility that people have. It can't just arbitrarily decide to make a phone call if it doesn't have a 'phone' dependency. Even when using Dependency Injection, you'll have to add that dependency to a business object. You'll have to explicitly write code to give it that capability, and even so, an injected dependency doesn't magically imbue a business object with the capability to make 'ad-hoc phone calls'. A dependency comes with specific methods you can call in order to answer specific questions.TryAcceptis a pure function, it's always composable. This means that my chance of being able to reuse it becomes much higher than if it's an object injected with various dependencies.Once you're adding code that enables an object to ask specific questions, you might as well just answer those questions up-front and pass the answer as method arguments. That's what this article's refactoring does. It knows that the
That's a brilliant observation! This just once again demonstrates what Evans wrote in DDD, that insight about the domain arrive piecemeal. A maître d'hôtel clearly doesn't depend on any repository, but rather on the book of reservations. You can add that as a dependency, or pass it as a method argument. I'd lean toward doing the latter, because I'd tend to view a book as a piece of data.MaîtreDobject is going to ask about the existing reservations for the requested date, so it just passes that information as part of an 'execution context'.Ultimately, if we are to take the idea of inversion of control seriously, we should, well, invert control. When we inject dependencies, we let the object with those dependencies control its interactions with them. Granted, those interactions are now polymorphic, but control isn't inverted.
If you truly want to invert control, then load data, pass it to functions, and persist the return values. In that way, functions have no control of where data comes from, or what happens to it afterwards. This keeps a software design supple.
Hi Mark, Thanks for your post, I think it's very valuable.
In the past, I had a situation when I was a junior software developer and just started working on a small, internal web application (ASP.NET MVC) to support HR processes in our company. At the time, I was discovering blogs like yours, or fsharpforfunandprofit.com and was especially fond of the sandwich architecture. I was preparing to refactor one of the controllers just like your example in this post (Controller retrieving necessary data from the repository, passing it to the pure business logic, then wrapping the results in a request). Unfortunately, My more experienced colleague said that it's a "fat controller antipattern" and that the controller can have only one line of code - redirecting the request to the proper business logic method. I wanted to explain to him that he is wrong, but couldn't find proper arguments, or examples.
Now I have them. This post is great for this particular purpose.
I guess it comes down to the amount of responsibilities the controller should have.
Marek named the fat controller antipattern. I remember reading about some years ago and it stuck, that's why I usually model my controllers to delegate the request to a worker class, maybe map a return value to a transfer object and wrap it all in some
ActionResult. I can relate to the argument that all I/O should happen at the boundaries of the system, though I'm not seeing it on the controller's responsibility list, all the more so when I/O exceeds a simple database call.I think that is what I was aiming for. The third type that Tyson is looking a name for could then be some kind of thin Data Access Layer, serving as a façade to encapsulate all calls to I/O, that can be injected into the
MaîtreDclass.Isn't code flexibility usually modeled using conditionals? Assume we are a very important guest and our maître d'hôtel really wishes to make a reservation for us, but all tables are taken. He could decide to phone all currently known guests to ask for a confirmation, if some guest cannot make it, he could give the table to us.
Using the initial version of
TryAccept, it would lead to something like this:public async Task<int?> TryAccept(Reservation reservation) { if(await CheckTableAvailability(reservation)) { reservation.IsAccepted = true; return await Repository.Create(reservation); } else { return null; } } private async Task<bool> CheckTableAvailability(Reservation reservation) { var reservations = await Repository.ReadReservations(reservation.Date); int reservedSeats = reservations.Sum(r => r.Quantity); if(Capacity < reservedSeats + reservation.Quantity) { foreach(var r in reservations) { if(!(await Telephone.AskConfirmation(r.Guest.PhoneNumber))) { //some guest cannot make it for his reservation return true; } } //all guests have confirmed their reservation - no table for us return false; } return true; }That is assuming that
MaîtreDhas a dependency on both the Repository and a Telephone. Not the best code I've ever written, but it serves its purpose. If the dependency onReservationis taken out of theMaîtreD, so could the dependency onTelephone. But then, you are deciding beforehand in the controller thatMaîtreDmight need to make a telephone call - that's business logic in the controller class and a weaker separation of concerns.And this is where I tend to disagree. The book of reservations in my eyes is owned and preciously guarded by the maître d'hôtel. Imagine some lowly garçon scribbling reservations in it. Unbelievable! Joking aside, the reservations in the book are pieces of data, no doubt about that - but I'd see the whole book as a resource owned by le maître and only him being able to request data from it. Of course, this depends on the model of the restaurant that I have in my mind, it might very well be different from yours - we didn't talk about a common model beforehand.
Apparently, I answered my own question when I moved the table availability check into its own private method. This way, a new dependency
TableAvailabilityCheckercan handle the availability check (complete with reservations book and phone calls), acting as a common data access layer.I have created a repository, where I tried to follow the steps outlined in this blog post with the new dependency. After all refactorings the controller looks like this:
public class ReservationsController : ControllerBase { private readonly MaitreD _maitreD; public ReservationsController(int capacity, IReservationsRepository repository, ITelephone telephone) { _maitreD = new MaitreD(capacity); Repository = repository; Telephone = telephone; } public IReservationsRepository Repository { get; } public ITelephone Telephone { get; } public async Task Post(Reservation reservation)
{
Reservation[] currentReservations = await Repository.ReadReservations(reservation.Date);
var confirmationCalls = currentReservations.Select(cr => Telephone.AskConfirmation(cr.Guest.PhoneNumber));
return _maitreD.CheckTableAvailability(currentReservations, reservation)
.Match(
some: r => new Maybe(r),
none: _maitreD.AskConfirmation(await Task.WhenAll(confirmationCalls), reservation)
)
.Match(
some: r => Ok(Repository.Create(_maitreD.Accept(r))),
none: new ContentResult { Content = "Table unavailable", StatusCode = StatusCodes.Status500InternalServerError } as ActionResult
);
}
}
During the refactorings, I was able to remove the
TableAvailabilityCheckeragain; I'm quite happy that the maître d'hôtel is checking the table availability and asking for the confirmations with the resources that are given to him. I'm not so happy with theTask.WhenAll()part, but I don't know how to make this more readable and at the same time make the calls only if we need them.All in all, I now think a bit differently about the controller responsibilities: Being at the boundary of the system, it is arguably the best place to make calls to external systems. If and how the information gathered from the outside is used however is still up to the business objects. Thanks, Mark, for the insight!
Thanks for writing this article. Doesn't testability suffer from turning the Maître d into an implementation detail of the ReservationsController? Now, we not only have to test for the controller's specific responsibilities but also for the behaviour that is implemented by the Maître d. Previously we could have provided an appropriate test double when instantiating the controller, knowing that the Maître d is tested and working. The resulting test classes would be more specific and focused. Is this a trade-off you made in favour of bringing the article's point across?
Max, thank you for writing. I don't think that testability suffers; on the contrary, I think that it improves. Once the
MaîtreDclass becomes deterministic, you no longer have to hide it behind a Test Double in order to be able to control its behaviour. You can control its behaviour simply by making sure that it receives the appropriate input arguments.The Facade Tests that cover
ReservationsControllerin the repository are, in my opinion, readable and maintainable.I've started a new article series about this topic, since I knew it'd come up. I hope that these articles will help illustrate my position.
Hi, Mark! Thank you for this blog post.
I really like the way of composing effectful and pure code the post explains. But here are some things I keep wondering about.
1) Is it correct that — given this approach — pure code cannot call back into impure code. When I say call back I mean it in a general way: maybe invoking a lambda passed as an argument to a function, maybe invoking a method on an injected dependency — basically the specific mechanics of calling back are irrelevant in this case.
2) In case point 1) is actually correct, have you ever had in your practice a task where the "ban" on callbacks was too limiting/impractical?
To give a more specific example of scenarios I have in mind, let's get back to
MaitreDfor a second. Let's imagine the amount of reservations data grew too big to load all at once. As a resultMaitreDneeds an instance ofReservationRepositoryso it can first run some business logic and based on the outcome read only a small specific subset of reservations from the repository.Or let's take a look at another imaginary scenario. Before confirming a reservation
MaitreDmust make a call to an external payment service to block a certain sum of money on the customer's card.These are only quick examples off the top of my head. Maybe dealing with them is easy, I would still be really grateful if you could give a couple of scenarios where you found it hard or impractical to do without "callbacks" and if and how you eventually manage to overcome the complications.
Mykola, thank you for writing. Yes, it's correct that a pure function can't call an impure function. This means, among other things, that you can't use Dependency Injection in functional programming.
Is that rule impractical? It depends on the programming language. In Haskell, that rule is enforced by the compiler. You can't break that rule, but the language is also designed in such a way that there's plenty of better ways to do things. In Haskell, that rule isn't impractical.
In a language like F#, that rule is no longer enforced, and there's also fewer built-in alternatives. The general solution to the problem is to use a free monad, but while it's possible to use free monads in F#, it's not as idiomatic, and there's a compelling argument to be made that going with partial application as Dependency Injection is more practical.
It's also possible to employ free monads in C#, but it's really non-idiomatic and hard to understand.
Haskell has some other general-purpose solutions (e.g. the so-called mtl style), but the only type of architecture I'm aware of that translates to F# or C# is free monads.
Does this mean that the ideas of this article is impractical for real software?
I don't think so. Let's consider your examples:
That's a frequently asked question, but in reality, I have a hard time imagining that. How much data is a reservation? It's a date (8 bytes), a quantity (in reality, a single byte is enough to keep track of that), as well as a name and an email address. Let's assume that an email address is, on average, shorter than 30 characters, and a name is shorter than 50 characters. We'll assume that we save both strings in UTF-8. Most characters are probably still just going to be 1 byte, but let's be generous and assume 2 bytes per character. That's 169 bytes per reservation, but let's be even more generous and say 200 bytes per reservation.What if we load 1,000 reservations? That's 200 kilobytes of memory. 10,000 reservations is 2 megabytes. That's about the size of an average web page. Is that too much data?
We routinely load web pages over the internet, and none but the Australians complain about the size.
My point is that I find it incredulous to claim that it'd be too much data if you need to load a couple of hundred of reservations in one go.
I can definitely imagine scenarios where you'd like to load reservations not only for the date in question, but also for surrounding dates. Even for a medium-sized restaurant, that's unlikely to be more than a few hundred, or perhaps a few thousand of reservations. That's not a lot of data. Most pictures on the WWW are bigger.
Just speculatively load extra data in one go. It's going to make your code much simpler, and is unlikely to affect performance if you're being smart about it. You can even consider to cache that data...
When it comes to your other question, I'll refer to a catch-phrase from the early days of large-scale web commerce (Pat Helland): take the money.
Don't make payment a blocking call. As soon as you have enough information to execute a purchase, kick it off as an asynchronous background job.
If a restaurant has that type of workflow that requires reservation of an amount on a credit card, you turn the business process into an asynchronous workflow. On the UI side, you make sure to reflect the current state of the system so that users don't try to reserve on dates that you already know are sold out. I show such a workflow in my functional architecture with F# Pluralsight course.
When a user makes a reservation, you take the reservation data and put it on a queue, and tell the user that you're working on it.
A background job receives the queued message and decides whether or not to accept the reservation. If it decides to accept it, it creates two other messages: one to reserve the money, and another a timeout.
Another background job receives the message to reserve the money on the credit card and attempts to do that. Once that's over, it reports success or failure by putting another message on a queue.
The reservation system receives the asynchronous message about the credit card and either commits or cancels the reservation. If it never gets such a message, the timeout message will eventually trigger a cancellation.
Each message handler can use the impure-pure-impure sandwich pattern, and in that way keep the business logic pure.
In my experience, you can often address issues like the ones you bring up by selecting an application architecture that best addresses those particular concerns. That'll make the implementation code simpler.
In fact, I'm often struggling to come up with an example scenario where something like a free monad would be necessary, because I always think to myself: Why would I do it that way? I'd just architect my application in this other way, and then the problem will go away by itself.
Hi Mark, I was just wondering - why is the controller asking its clients for a capacity, when it only uses it to imediately create a MaitreD? Should it not ask for the MaitreD in the first place, instead of sneakily conjuring it up in the constructor?
It feels like the controller owns the MaitreD, but the MaitreD represents pure and deterministic behaviour set up by its "capacity" dependency - and the capacity is only injected into the controller. So should it really be private?
Can't we just ask for the MaitreD, leave it public, forget about the "implementation detail" of a capacity, and be done with it?
Thanks!
votroto, thank you for writing. Injecting a concrete dependency is definitely an option. I'm currently working on a larger example code base that also has a
MaitreDclass, and in that code base I decided to inject that into the Controller instead of its constituent elements.It's a trade-off; I don't see one option as more correct than the other. In the present article, the
MaîtreDclass is so simple that it only has a single dependency:capacity. In this case, it's a toss-up. Either you injectcapacity, or you inject the entireMaîtreDclass. In both cases, you have a single dependency in addition to thatIReservationsRepositorydependency. In this situation, I chose to follow the principle of least knowledge. By injecting only thecapacity, theMaîtreDclass is an implementation detail not exposed to the rest of the world.In the larger example code base that I'm currently working on, the
MaitreDclass is more complex: it has several configuration values that determine its behaviour. If I wanted to keep theMaitreDclass an implementation detail, the Controller constructor would look like this:I don't think that this addresses any real concerns. If I ever decide to change the
MaitreDconstructor, I'd have to also change theReservationsControllerconstructor. Thus, whileReservationsControllermight not 'formally' depend onMaitreD, it still does so in practice. In that case I chose to injectMaitreDinstead:Don't read the above as an argument for one option over the other. I'm only trying to explain the deliberations I go through to arrive at a decision, one way or the other.