[Comment] ASP.NET Core Lazy Command Pattern

[RehanSaeed]

https://rehansaeed.com/asp-net-core-lazy-command-pattern/

[RehanSaeed]

Mike-EEE commented on 2017-04-08 12:45:46

Command pattern is indeed awesome! Thank you for promoting it. As you may know, it was a staple in WPF development.

Quick commentary, however. ;) What used to be a simple:

void ICommand.Execute(T parameter) 

... in WPF is now:

Task IAsyncCommand.ExecuteAsync(T parameter)

... in .NET Core. So, 38 characters "old" way vs. 62 characters "new" way. So much for progress. ;) As you are using an expression body for the constructor I am sure you can appreciate the sentiment that less code is better.

https://visualstudio.uservoice.com/forums/121579-visual-studio-ide/suggestions/9126493-improve-asynchronous-programming-model

Additionally, this design muddies the water with command and query separation, essentially forcing you to enable every "query" method (anything that returns a result) into a command method, which implicitly changes state and has side effects (which query methods are not supposed to have).

Food for thought. :)

[RehanSaeed]

Mike-EEE commented on 2017-04-08 12:49:17

Command pattern is indeed awesome! Thank you for promoting it. As you may know, it was a staple in WPF development.

Quick commentary, however. ;) What used to be a simple:

void ICommand.Execute(T parameter) 

... in WPF is now:

Task IAsyncCommand.ExecuteAsync(T parameter)

... in .NET Core. So, 38 characters "old" way vs. 62 characters "new" way. So much for progress. ;) As you are using an expression body for the constructor I am sure you can appreciate the sentiment that less code is better.

https://visualstudio.uservoice.com/forums/121579-visual-studio-ide/suggestions/9126493-improve-asynchronous-programming-model

Additionally, this design muddies the water with command and query separation, essentially forcing you to enable every "query" method (anything that returns a result) into a command method, which implicitly changes state and has side effects (which query methods are not supposed to have).

Food for thought. :)

Boo, the blog formatted out the brackets. Let's see if JSON-based formatting works (sigh):

void ICommand{T}.Execute(T parameter)

Task{IActionResult} IAsyncCommand{T}.ExecuteAsync(T parameter)

It's actually 37 "old" vs 62 "new."

[RehanSaeed]

Manuel Grundner commented on 2017-04-08 14:20:58

I totally understand the SRP and the IOC Concept behind this, but whats then the purpose of the Controller? Besides declaring The Routes? I mean in a more reallife Example?

[RehanSaeed]

Muhammad Rehan Saeed commented on 2017-04-08 16:40:34

I totally understand the SRP and the IOC Concept behind this, but whats then the purpose of the Controller? Besides declaring The Routes? I mean in a more reallife Example?

I think of a controller as a kind of specialized interface for HTTP or maybe a better way to describe it would be a contract. It has all your routes, caching and authorization attributes etc. It defines the public contract for how you want to do HTTP.

In an imaginary world where each action method was turned into an action command class, it'd be quite difficult to get a quick overview of all of your HTTP endpoints, although such an architecture might be cool.

[RehanSaeed]

Adie commented on 2017-04-09 15:50:02

Nice idea, better sample with asp identity would be nice

[RehanSaeed]

Muhammad Rehan Saeed commented on 2017-04-09 16:03:11

Nice idea, better sample with asp identity would be nice

Agreed. ASP.NET Identity is a mess and a prime example of code that could use the command pattern. Not enough time in my day to show that though, so that's an exercise for the reader.

[RehanSaeed]

Paul commented on 2017-04-11 14:38:04

I use Jimmy Bogard's MediatR project to accomplish a similar purpose where controllers simply execute commands and queries. I love that pattern!

public class Ping : IRequest
{
    public string Name { get; set; }

    public class Pong 
    {
        public string Message { get; set; }
    }

    internal class Handler : IRequestHandler 
    {     
        public Pong Handle(Ping request)
        {
            return new Pong { 
                Message = "Ping: " + request.Name
            };
        }
    }
}

[RehanSaeed]

Theo Albers commented on 2017-04-13 09:14:44

I have used this pattern as well to keep the API controller lean. I decided to move the "protocol" out of the command and into the controller action. So the command returns an object and the controller action returns the wire result NotFoundResult/OkObjectResult.

The question that keeps spinning in my head: why not create ApiControllers with a single method? So instead of RocketController have RocketGetController, RocketLaunchController. That way the controller turns into a command/action.

[RehanSaeed]

Juan Perez commented on 2017-04-18 17:31:38

I think of a controller as a kind of specialized interface for HTTP or maybe a better way to describe it would be a contract. It has all your routes, caching and authorization attributes etc. It defines the public contract for how you want to do HTTP.

In an imaginary world where each action method was turned into an action command class, it'd be quite difficult to get a quick overview of all of your HTTP endpoints, although such an architecture might be cool.

I do not totally agree. If you organize the commands in folder or namespaces and use names related to HTTP methods, it could be better and simpler. It is almost like a class for each action.

We could create controllers that implement command interface and we could avoid all these ugly Lazy fields.

[RehanSaeed]

Muhammad Rehan Saeed commented on 2017-04-21 15:32:39

I have used this pattern as well to keep the API controller lean. I decided to move the "protocol" out of the command and into the controller action. So the command returns an object and the controller action returns the wire result NotFoundResult/OkObjectResult.

The question that keeps spinning in my head: why not create ApiControllers with a single method? So instead of RocketController have RocketGetController, RocketLaunchController. That way the controller turns into a command/action.

Controllers with a single action method are interesting and certainly a possible direction. I personally feel that an action method is doing two things:

1. Defines an interface for a request.
2. Builds and returns a response. These two things go against the single responsibility principle. When you are working on building a reponse, you don't care about the request interface (routes, auth etc.), thats all noise. A controller per action method would also make it more difficult to see all your related routes in one place. Using the command pattern, the controller just becomes a specialized interface for HTTP.

[RehanSaeed]

Muhammad Rehan Saeed commented on 2017-04-21 15:33:18

I do not totally agree. If you organize the commands in folder or namespaces and use names related to HTTP methods, it could be better and simpler. It is almost like a class for each action.

We could create controllers that implement command interface and we could avoid all these ugly Lazy fields.

See my answer to Theo Albers.

[RehanSaeed]

paul commented on 2017-04-29 23:44:52

Controllers with a single action method are interesting and certainly a possible direction. I personally feel that an action method is doing two things:

1. Defines an interface for a request.
2. Builds and returns a response. These two things go against the single responsibility principle. When you are working on building a reponse, you don't care about the request interface (routes, auth etc.), thats all noise. A controller per action method would also make it more difficult to see all your related routes in one place. Using the command pattern, the controller just becomes a specialized interface for HTTP.

I agree with Theo that I cannot see how this approach offers any benefit over 'one action controllers' and in fact it just adds complexity. You mentioned that the controller defies the interface for the request but this is not strictly true. The route is only part of the interface. The request and response types are also a significant part of the contract and these are defined in the command. So now we have the command being responsible for some of 1) and all of 2). Just my two cents.

[RehanSaeed]

XperiAndri commented on 2017-07-15 15:28:28

Looks interesting however very verbose and overcomplicated. Solutions with MediatR look much more concise and elegant.

And in my opinion your current implementation look like moving controller specific stuff to business layer.

[RehanSaeed]

Muhammad Rehan Saeed commented on 2017-07-23 12:37:56

Looks interesting however very verbose and overcomplicated. Solutions with MediatR look much more concise and elegant.

And in my opinion your current implementation look like moving controller specific stuff to business layer.

MediatR is interesting but it's another dependency, and much slower than this implementation because it uses Activator.CreateInstance(). This implementation is much simpler (a couple of interfaces used by the DI container, has zero dependencies and is as fast as it gets.

I debated with myself whether the controller should return a business object or an IActionResult. I actually started with the former but settled on the latter. It makes things much simpler, it's all C# POCO's which makes unit testing a breeze.

[RehanSaeed]

Manish commented on 2017-08-22 03:51:20

I have used this pattern as well to keep the API controller lean. I decided to move the "protocol" out of the command and into the controller action. So the command returns an object and the controller action returns the wire result NotFoundResult/OkObjectResult.

The question that keeps spinning in my head: why not create ApiControllers with a single method? So instead of RocketController have RocketGetController, RocketLaunchController. That way the controller turns into a command/action.

Hello, How did you do it? Can you please post sample code?

[RehanSaeed]

Aluan Haddad commented on 2018-01-20 11:13:20

While abstracting away common patterns is of course good, this overlooks the basic facilities the language already provides to accomplish this.

The IAsyncCommand interfaces you have defined introduce unnecessary complexity and represent a barrier to simple composition. In particular, the whole pattern reeks of Java and the complexity and ugliness inherent in its abhorrent SAM types.

This is C#, we don't need nonsensical, SAM types, we have delegates.

[TarasKuz]

Instead of your lazy stuff use FromServicesAttribute for each action/command

[RehanSaeed]

Instead of your lazy stuff use FromServicesAttribute for each action/command

@TarasKuz Agreed, soon after I wrote this, I switched to using [FromServices].