Is it possible to have a create and update command that uses the same command type?

[ghost]

Hi,

I started playing round with the command query pattern and wanted it to work in a crud like manner. So I started by prototyping a create and update commands that would use the same DTO command but this then confuses Simple Injector as it doesn't know which one to instantiate as they use the same command. In my mind it seems that the create and update commands would use the same DTO and the only way to get around this I can think of is to use inheritance for separate commands or to define to separate types with the same properties?

Bellow is some basic code demonstrating what I've done. Starting with the DTO that is used as my commands.

class AddressDto
{
    public int Id { get; set; }
    public string Line1 { get; set; }
    public string Line2 { get; set; }
    public string Postcode { get; set; }
}

Both of my handlers

class CreateAddressHandler : ICommandHandler<AddressDto>
{
    public void Handle(AddressDto command)
    {
        //Do something
    }
}
class UpdateAddressHandler : ICommandHandler<AddressDto>
{
    public void Handle(AddressDto command)
    {
        //Do something
    }
}

The creation of the handler

class Program
{
    static readonly Container container;

    static Program()
    {
        container = new Container();

        container.Register(typeof(ICommandHandler<>), AppDomain.CurrentDomain.GetAssemblies());
    }
    static void Main(string[] args)
    {
        var service = container.GetInstance<CreateAddressHandler>();
        AddressDto command = new AddressDto();
        //Fill up our command and then use it
        service.Handle(command);
    }
}

[dotnetjunkie]

this then confuses Simple Injector as it doesn't know which one to instantiate as they use the same command.

This is not so much that Simple Injector is confused, but that your design is ambiguous.

These query/handler and command/handler patterns are a form of a more general message based architecture or type based programming. With type based programming it's not the method of a class that describes what should happen, but it is the type.

In your case you have a type AddressDto, but it doesn't describe what it should do. It could be a create, update, delete, perhaps even something else. You will have to make this explicit using types.

There are several ways to do this. In its simplest form, just create 2 types:

class CreateAddress { public AddressDto Address { get; set; } }
class UpdateAddress { public AddressDto Address { get; set; } }

class CreateAddressHandler : ICommandHandler<CreateAddress> { ... }
class UpdateAddressHandler : ICommandHandler<UpdateAddress> { ... }

Notice how this example uses Composition over Inheritance. As a general rule of thumb, don't use inheritance at all. If you think you need it, take a step back, composition is almost always the better solution. The only reason I do inheritance nowadays is when I need to interact with some framework (MVC for instance forces us to inherit from Controller base class).

There are options, but in most cases I would go with this. It's the simplest thing to do, and this design has other advantages. For instance, it becomes trivial to give each command its own permission or role:

[Permission(Permissions.CreateAddress)]
class CreateAddress { public AddressDto Address { get; set; } }

[Permission(Permissions.UpdateAddress)]
class UpdateAddress { public AddressDto Address { get; set; } }

This gives you fine-grained control over every operation in the system.

Do note however, that in most cases, you should prevent having a very CRUDdy system. The problem with CRUD is that you are losing a lot of intent. You want to know why some operation is executed, while CRUD only talks about data. This makes it very hard to run more complex business logic behind it different whys.

In your case for instance, why exactly is the address updated? There could be a huge difference between correcting a typo in an address, and whether a user actually moved to a different address. It's very likely that quite some different business processes have to be started when a user is moved. In a CRUD system you completely lose that information, which is typically bad.

So instead, you might even want to have commands like this:

[Permission(Permissions.RegisterCustomer)]
class RegisterCustomer { 
    public AddressDto { get; set; } 
    public CustomerInfo { get; set; }
}

[Permission(Permissions.FixCustomerAddressTypo)]
class FixCustomerAddressTypo {
    public Guid CustomerId { get; set; }
    public AddressDto { get; set; } 
}

[Permission(Permissions.MoveCustomer)]
class MoveCustomer {
    public Guid CustomerId { get; set; }
    public AddressDto { get; set; } 
}

There are other options though, especially when you stick with CRUD. You can create an envelope to specify whether we're dealing with creates or updates:

class Create<TData> { public TData Data { get; set; } }
class Update<TData> { public TData Data { get; set; } }

This is especially useful when your handler is generic as well:

class CreateHandler<TData> : ICommandHandler<Create<TData>>
{
    public void Handle(Create<TData> command) { ... }
}

class UpdateHandler<TData> : ICommandHandler<Update<TData>>
{
    public void Handle(Update<TData> command) { ... }
}

Does this help?

[ghost]

This has been extremely helpful I can't believe I didn't think of using composition.

[ghost]

One thing I've noticed with using composition though is that I may not be using all of the properties of the a DTO class and that could mean I should be making even more use specific DTO's, or am I just being pedantic and that really doesn't matter?

[dotnetjunkie]

Can you show an example?

[ghost]

Sure, but sorry it's now in VB.

Here is my command.

Public Class CreateProcedureTemplateDetailCommand
    Implements ICommand

    <Required>
    Public Property HeaderId As Integer
    <Required>
    Public Property Order As Integer
    Public Property Detail As ProcedureTemplateDetailDTO

End Class

This is a DTO that pretty much copies the entity class in entity framework but is just used by the service layer.

Public Class ProcedureTemplateDetailDTO
    Public Property Id As Integer
    Public Property DisplayValue As String 'For command this is filled
    Public Property TypeId As Integer 'For command this is filled
    Public Property Type as String
    Public Property Reportable As Boolean 'For command this is filled
    Public Property Validate As Boolean 'For command this is filled
    Public Property Order As Integer

    Public Property Questions As List(Of QuestionItemDTO)
    Public Property Groupings As List(Of GroupItemDTO)
End Class

You can see there is quite a few properties in our ProcedureTemplateDetailDTO that does not need to be filled. Should I just extract them and just put them in the command instead or just leave it as it is? Let me know if more details are needed?

[ghost]

I could even refactor this more using composition like this. The Db structure would be a header has many stages and each stage is made up of many details.

Public Class CreateProcedureTemplateDetailCommand
    Implements ICommand

    Public Property Header As ProcedureTemplateHeaderDTO
    Public Property Stage As ProcedureTemplateStageDTO
    Public Property Detail As ProcedureTemplateDetailDTO

End Class

Public Class ProcedureTemplateHeaderDTO
    Public Property Id As Integer 'Only This would be used by command
    Public Property Name As String

    Public Property ProcedureTemplateStages As List(Of ProcedureTemplateStageDTO)
End Class

Public Class ProcedureTemplateStageDTO

    Public Property Id As Integer
    Public Property Name As String
    Public Property Order As Integer
    Public Property ConsultantEditable As Boolean

    Public Property Details As List(Of ProcedureTemplateDetailDTO)
End Class

[dotnetjunkie]

The solution is not that clear cut. In general though I would say:

• Specify DTOs especially for your commands.
• Use composition to build up your messages.

You can't really say that this is code duplication:

Public Class ProcedureTemplateDetailCommandDTO
    Public Property DisplayValue As String
    Public Property TypeId As Integer
    Public Property Reportable As Boolean
    Public Property Validate As Boolean
End Class

or

Public Class UdpateProduceTemplateDetailsCommand
    Public Property DisplayValue As String
    Public Property TypeId As Integer
    Public Property Reportable As Boolean
    Public Property Validate As Boolean
End Class

If you apply composition, you can also do the following:

Public Class UdpateProduceTemplateDetailsCommand
	Public Property Details As ProcedureTemplateDetail
End Class

Public Class ProcedureTemplateDetail
    Public Property DisplayValue As String
    Public Property TypeId As Integer
    Public Property Reportable As Boolean
    Public Property Validate As Boolean
End Class

Public Class ProcedureTemplateDetailInfo
    Public Property Id As Integer
    Public Property Type as String
    Public Property Order As Integer
    Public Property Details As ProcedureTemplateDetail

    Public Property Questions As List(Of QuestionItemDTO)
    Public Property Groupings As List(Of GroupItemDTO)
End Class

This allows you to use ProcedureTemplateDetailInfo as return type of queries, while using ProcedureTemplateDetail as DTO for commands.

But be careful in overdoing this, it can be beneficial to have a little bit of 'duplication', with commands having their own set of properties. You'll often see that a client that executes a query doesn't use all those properties. When that happens, it really deserves a different DTO. Query results are typically use case specific. Prevent making very generic queries with very generic return types that can be reused all over the place. Instead let your queries focus on a specific use case. This typically makes the overall system simpler and makes it much easier to performance tune the query handlers (and the database) later on.

[ghost]

Some of the things that I'm taking away from this is I should use DTO's that are used mostly exclusively by queries and commands?

Use composition to make up the data that is needed for that specific case. In your case you used the info type but that would contain most of the data but that wouldn't necessarily be useful for every case and should be tuned to specific cases.

Here is some refactoring I've done to demonstrate this and these would be used by certain queries.

'These are for generic table columns
Public Class ProcedureTemplateHeaderDTO
   Public Property Id As Integer
   Public Property Name As String
End Class

Public Class ProcedureTemplateStageDTO

   Public Property Id As Integer
   Public Property Name As String
   Public Property Order As Integer
   Public Property ConsultantEditable As Boolean
End Class

'These would be our specfic use case say for certain screens.
Public Class ProcedureTemplateHeaderWithStagesDTO
   Public Property Header As ProcedureTemplateHeaderDTO
   Public Property Stages As List(Of ProcedureTemplateStageDTO)
End Class

Public Class ProcedureTemplateStageWithDetailsDTO
   Public Property Stage As ProcedureTemplateStageDTO
   Public Property Details As List(Of ProcedureTemplateDetailDTO)
End Class

[dotnetjunkie]

Some of the things that I'm taking away from this is I should use DTO's that are used mostly exclusively by queries and commands?

I would even say: mostly exclusively by queries or commands.