Using the Command-Query Separation principle in MVC Controllers
Asked Answered
H

3

6

I like the idea of Command Query Separation but can't see how to use it within an MVC Controller action which is adding an entity, and needs the new entity's ID after adding it.

For example, in the simplified example below a service is used to create a new item:

public ActionResult Assign(AssignViewModel viewModel)
{
     var newItem = _AssignItemService.AssignItem(viewModel.ItemName, viewModel.ItemValue);

     return RedirectToAction("ListItem", new {id = newItem.Id);
}

But when I redirect to the action which is going to display the new item, I need to know the ID of the newly created item, in order that it can be retrieved from the database. So I have to ask the service to return the newly created item (or at least, its ID).

In pure CQS, a command has no return value, so the pattern above would be invalid.

Any advice gratefully received.

Hedwighedwiga answered 3/5, 2012 at 14:45 Comment(0)
I
4

You should pass to AssignItem method an instance of "Item" (or whatever your entity's name is) that is created from the viewmodel's values, then the method doesnt have to return anything, instead it will just update entity's Id property making it a Command method.

You can then use entity.Id for anything you want

Inventory answered 3/5, 2012 at 15:2 Comment(7)
How do you retrieve "item" later?Corettacorette
the object that is passed to AssignItem method contains the Id information once it is returned. As the method is a command it is ok to change the object's stateInventory
Aren't you basically still returning a value at that point? Sure, it's not coming out of the back of the function, but you're still returning a value, you're just pushing the return value somewhere else. That said, I like the idea of keeping the ID within the object, rather than returning it separately.Corettacorette
That difference is the main reason why command query separation is a good idea most of the time. If you call many times that same method with those same arguments it will return diferent things each time which is odd. Instead if you pass the same object again and again it will always return nothing and only change object's state when it is necesary (usually just the first time since thats a db insert)Inventory
You would never pass the same object for insertion twice. That the method returns something different each time is by design; each new ID must be unique to distingish it from the others.Corettacorette
maybe you want to change item's name, maybe you don´t for this specific scenario but you will for another, if you want to make an idiom out of this you need to be consistent. Anyway, it was a nice talk, as I said I agree with you that nothing is absolut anyway, cheersInventory
I think I will try to use your suggestion in the future. There is a complication I forgot about when asking the question, which is that the Unit of Work doesn't necessarily get committed during the call to the AssignItemService - it could happen later. So the ID of the new entity might not even be available as a return value. In this case, passing in a parameter which is updated with the ID when the item is actually saved to the database seems like the best approach. Thanks for your ideas.Hedwighedwiga
C
8

I think you're stuck on a pedantic point.

A query is when you want to ask the database a question, like "How many customers west of the Mississippi purchased red colored items during the month of June?" That's a query. Returning the ID during an insert is not a typical query, per se.

As with most other things in software development, this pattern is not an absolute. Even Fowler says he's willing to break it when it is convenient to do so:

Popping a stack is a good example of a modifier that modifies state. Meyer correctly says that you can avoid having this method, but it is a useful idiom. So I prefer to follow this principle when I can, but I'm prepared to break it to get my pop.

If you really want to retrieve the most recently-added ID from a database separately from its insertion, you can use something like Scope Identity. But I think you're adding complexity for no additional benefit.

Corettacorette answered 3/5, 2012 at 14:53 Comment(7)
Also, make sure that if you use Scope identity or similar that you handle multi-threaded situations properly. Eg T1 inserts, T2 inserts, T1 gets Id from T2s insert, T2 gets the same Id. Of course, the same problem applies to any atomic operation/transaction so I'm sure you've considered this.Arnulfo
Another good reason to simply let the database hand you the ID through the insert function's return value.Corettacorette
I think Fowler's comment take into account that pop method in a stack is ok since its use is general so it has become an idiom for all developers, but I don't consider that returning an id from a save method is an idiom. I agree with you though that nothing is absolute and it is not a big deal to break this kind of rules once in a whileInventory
@jorgehmv: It would be an insert method, and returning an ID from the database is a very common and very well understood idiom.Corettacorette
@Robert ok maybe it is, but it was also a well understood idiom to return error codes from methods many years ago, this is not a good practice anymore, martinfowler.com/refactoring/catalog/…Inventory
@jorgehmv: An error code is not the same thing, we have exceptions for that; you might as well say returning any value from a function is bad practice.Corettacorette
Thanks all for the answers & debate. I'm quite happy to use a return value (which is how I've done things in the past). I think I must be missing the point/benefits of CQS though - my scenario must be pretty common, so why is CQS seen as a "good thing" if you need an exception for such a basic case? I'm not hung up on applying CQS - just trying to understand it.Hedwighedwiga
I
4

You should pass to AssignItem method an instance of "Item" (or whatever your entity's name is) that is created from the viewmodel's values, then the method doesnt have to return anything, instead it will just update entity's Id property making it a Command method.

You can then use entity.Id for anything you want

Inventory answered 3/5, 2012 at 15:2 Comment(7)
How do you retrieve "item" later?Corettacorette
the object that is passed to AssignItem method contains the Id information once it is returned. As the method is a command it is ok to change the object's stateInventory
Aren't you basically still returning a value at that point? Sure, it's not coming out of the back of the function, but you're still returning a value, you're just pushing the return value somewhere else. That said, I like the idea of keeping the ID within the object, rather than returning it separately.Corettacorette
That difference is the main reason why command query separation is a good idea most of the time. If you call many times that same method with those same arguments it will return diferent things each time which is odd. Instead if you pass the same object again and again it will always return nothing and only change object's state when it is necesary (usually just the first time since thats a db insert)Inventory
You would never pass the same object for insertion twice. That the method returns something different each time is by design; each new ID must be unique to distingish it from the others.Corettacorette
maybe you want to change item's name, maybe you don´t for this specific scenario but you will for another, if you want to make an idiom out of this you need to be consistent. Anyway, it was a nice talk, as I said I agree with you that nothing is absolut anyway, cheersInventory
I think I will try to use your suggestion in the future. There is a complication I forgot about when asking the question, which is that the Unit of Work doesn't necessarily get committed during the call to the AssignItemService - it could happen later. So the ID of the new entity might not even be available as a return value. In this case, passing in a parameter which is updated with the ID when the item is actually saved to the database seems like the best approach. Thanks for your ideas.Hedwighedwiga
J
0

The way to do that would be to make the caller specify the ID for the new entity (which most likely implies using GUIDs as the key).

However, in my experience, imposing the (purist) rule that a command may not return a result is going to cause problems for little gain.

Jumada answered 3/5, 2012 at 15:7 Comment(1)
Thanks erikkallen. Makes sense, but in the end I think jorgehmv's idea will work better for me.Hedwighedwiga

© 2022 - 2024 — McMap. All rights reserved.