How should separated methods be used?

Question

Among other things, the CQRS has a rule that each of the methods has one responsibility. So we can't have this in the model:

public class Store {
    public Item getOrCreateItem(int id)
    {
        //logic
    }
     //....
}

and this in the controller:

Store store = new Store();
Item item = store.getItem(1);

We must have this in the model:

public class Store {
    public Item createItem()
    {
        //logic
    }
    public Item getItem(int id)
    {
        //logic
    }
     //....
}

which leads to this in the controller:

Store store = new Store();
Item item = store.getItem(1);
if (item !== null) {
    Item item = store.createItem();
}

But now we have a lot of code in the controller...

Is there any better way to seperate the methods than to have the extra code in the controller, which should contain little code?

Possible duplicate of [Is it a good practice to write a method that gets something and checks the value?](https://softwareengineering.stackexchange.com/questions/198337/is-it-a-good-practice-to-write-a-method-that-gets-something-and-checks-the-value) — gnat, Feb 22 '18 at 21:22
According to some anecdote, [// *Meyer likes to use command-query separation absolutely, but there are exceptions. Popping a stack is a good example of a query 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.* //](https://martinfowler.com/bliki/CommandQuerySeparation.html) — rwong, Feb 22 '18 at 23:00
In this case, it is indeed doing more than an "add" or a "get" - its essence is to "sync all information that either side (store or controller) has knowledge of this item", where "sync" is bidirectional. Any discrepancy lead to reproducible bugs. Thus, "sync" is an example of operation that cannot be adequately modeled by CQRS. Similarly, "sync" is also an example that cannot be adequately modeled by ["Single source of truth"](https://en.wikipedia.org/wiki/Single_source_of_truth), because both the Store and the Controller wants to have access to the truth, but that can't happen. — rwong, Feb 22 '18 at 23:03
The question title can say "How to rewrite AddOrGetItem so that it is compatible with CQRS idiom?" because the question successfully highlights the incompatibility and has a good chance of inviting insights and extensions (or, refutation) to the CQRS idiom. — rwong, Feb 22 '18 at 23:34

candied_orange · Accepted Answer · 2018-02-23T11:23:14.083

20

Oh for crying out loud.

The controller has no business knowing what your implementation of getThingy() has behind it. Whether getThingy() creates or passes out a cached copy is none of it's business. The only reason this implementation detail leaked out of the store and into the controller was because you didn't trust the store to just "somehow" get you a thingy. Once you start thinking you know "how", from outside, suddenly you're trapped doing what's expected. That's not abstraction.

If you're thinking "but getters aren't supposed to be factories" then fine. Name it thingy().

If you're thinking "but I have other things that need to get thingy different ways" then fine. Name it controllersThingy().

If all I ask for is a thingy then just give me my thingy. I don't care how. Stop telling me how.

edited Feb 23 '18 at 11:23

answered Feb 22 '18 at 22:28

candied_orange

102,279
24
197
315

In other words, @losasafo, what CandiedOrange is saying is that your controller is using both `Store.createItem` and `Store.getItem` to *get* your item. Your controller is using both of them for the *same exact purpose*. In this case, yeah, it should be able to just call `Store.getItem`, and the model can create the item first if it needs to. It's still *getting* the item either way. If you really do have a separate need for your model to explicitly create the item, at which point your model is doing something really different with item creation, that's a different situation. – Panzercrisis Feb 23 '18 at 16:31
Something similar to the example in your question is "upserting": That's when you try to save something to a database or whatever, and you don't particularly care if it's already in there or not; you just want its current state to be saved either way. So you pass in the information to save from one layer or client, and the server or other layer will save it. To save it, it will create it brand new if necessary, or if there's already an old record of it in there, it will just update it instead. Either way, you don't care, so you "upsert" it. – Panzercrisis Feb 23 '18 at 16:50
1

@panzercrisis Hehe, yes [upsert](https://en.wikipedia.org/wiki/Merge_(SQL)) is a good example of abstracting away details you don't care about. – candied_orange Feb 23 '18 at 17:26

score 2 · Answer 2 · answered Feb 22 '18 at 21:03

Typically, one would have a Service which would provide this sort of functionality. The Controller would call something like StoreService.EnsureItemExists(1).

The model should probably not do much either - most of the business logic should reside in the service.

How should separated methods be used?

2 Answers2

Linked