Should command info be passed to aggregate using method parameters or by reference?

Question

I'm working on implementing a basic CQRS+ES application. I've seen many examples but I don't understand the routing between the command handler and aggregate.

In some examples, the work is did in this way:

XCommandHandler:

void Handle(XCommand command) {
   var aggregate = this.repository.Find<Aggregate>(command.aggId);

   aggregate.InvokeSomeBusinessLogic(command.property1, command.property2);
   this.repository.Save(aggregate);
}

But others do in another way:

XCommandHandler:

void Handle(XCommand command) {
   var aggregate = this.repository.Find<Aggregate>(command.aggId);

   aggregate.InvokeSomeBusinessLogic(command);
   this.repository.Save(aggregate);
}

What is the best approach, especially when you have many properties (15 or more) in a command?

score 10 · Accepted Answer · edited Feb 23 '17 at 09:10

10

This isn't particularly a question about CQRS+ES, but just about API design overall. As Clean Code explains, a method with no parameters is better than a method with one parameter, which is better than one with two parameters, etc.

Blindly following that rule should always make you select your second option:

aggregate.InvokeSomeBusinessLogic(command);

However, blindly following any rule is rarely a good idea. In this case, it would be a good idea to consider the Interface Segregation Principle as a countering force:

Clients should not be forced to depend on methods they do not use.

Since getters and setters are methods too, I think this principle applies to Command Objects as well.

Thus, if you have 15 or more properties, but the method in question only needs two of them, perhaps it would be better to just pass those two values.

Another thing entirely is that if you have a Command Object with 15 properties, you may want to rethink your design. One of the fundamental assumptions behind CQRS+ES is that applications present task-based UIs, which ensures that each Command (and the corresponding Events) are 'small'.

Editing in Mark's comments with links because one of the links were broken:

Updating aggregate info isn't task-based; it's CRUD. Perhaps these two articles from Udi Dahan will be helpful.

Queries, Patterns, and Search – food for thought

Tasks, Messages, & Transactions – the holy trinity

edited Feb 23 '17 at 09:10

kayess

3,384
9
28
45

answered Jan 21 '15 at 17:10

Mark Seemann

225,310
48
427
736

Thank you Mark, but if the UI-Task is related to update aggregate info?, UI captures the aggregate main info, and put in the Bus (or Channel like your CQRS example), so I have a command with these 15 properties all related to the same task, update the aggregate. I should split this logic in more small commands? or is valid have a update command with all data inside it? – Mr Rivero Jan 21 '15 at 17:19
Updating aggregate info isn't *task-based*; it's CRUD. Perhaps these two articles from Udi Dahan will be helpful. http://www.udidahan.com/2007/03/31/tasks-messages-transactions-%E2%80%93-the-holy-trinity http://www.udidahan.com/2013/04/28/queries-patterns-and-search-food-for-thought – Mark Seemann Jan 21 '15 at 17:37
Perfect. I got it! I'm modeling the Domain Model in a wrong way, I'll do in a new manner. – Mr Rivero Jan 21 '15 at 18:36
One question based on the topic please: Is the following a bad practice? var aggregate = this.repository.SomeMethod(command) ? – fdhsdrdark Aug 15 '23 at 07:00
@fdhsdrdark Hard to tell, but if `command` represents a Command, then why does `SomeMethod` return anything? – Mark Seemann Aug 15 '23 at 07:24
True, it should normally not return sth since it's a command. But, if it had to, returning an Aggreagate is the way to go? Maybe a commandResult object would be more appropriate? – fdhsdrdark Aug 15 '23 at 07:53
@fdhsdrdark [Mu](https://en.wikipedia.org/wiki/Mu_(negative)#Non-dualistic_meaning). If you post a new question with more context, I'll be happy to take a look if you ping me. – Mark Seemann Aug 15 '23 at 08:19
@MarkSeemann Will do! Thank you very much! – fdhsdrdark Aug 15 '23 at 08:23
For anyone reading up to this comment, below is a discussion that helped me clear things out. https://stackoverflow.com/questions/43433318/cqrs-command-return-values @MarkSeemann Thank you very much for beeing willing to help. – fdhsdrdark Aug 27 '23 at 07:59

score 3 · Answer 2 · answered Jan 22 '15 at 13:48

I would agree with Mark in principle. I would note however that CQRS has grown from the domain driven design world. They stress the importance of language (ubiquitous language). You can capture the intent and meaningfulness in the actual name of the command object.

On a practical level refactoring methods by adding parameters can become a real pain if called from more than a few places. This can be significantly eased with a single object.

I have a couple of related posts that you may find helpful. Aggregate Root – How to Build One for CQRS and Event Sourcing and 6 Code Smells with your CQRS Events – and How to Avoid Them. While the last one is about events many of the principles apply.

Hope you find them helpful.

Thank you, I made a refactoring to our code, following Mark suggestions, and the code is more maintainable just now. — Mr Rivero, Jan 26 '15 at 15:30

Should command info be passed to aggregate using method parameters or by reference?

2 Answers2