search

jdaniel1987 / GameShop.CQRS

Example of CQRS
4 stars 0 forks source link

Decouple API response from Application response #1

Closed MrDave1999 closed 2 months ago

MrDave1999 commented 2 months ago

I have reviewed this commit: https://github.com/jdaniel1987/GameShop.CQRS/commit/3857c8e157bbf2aee8649a50b1cbdc0a62340649 and it not only adds more complexity, but it makes your queries slower because you have to do a lot of memory reallocations, so there is more load on the garbage collector.

https://github.com/jdaniel1987/GameShop.CQRS/blob/47adfafb92ef125560ff3e1d7064d12938589342/GameShop.Application/Extensions/GameConsoleExtensions.cs#L57-L77

You are doing three mappings: GameConsole -> GetAllGameConsolesQueryResponse -> GetAllGameConsolesResponse

jdaniel1987 commented 2 months ago

I have reviewed this commit: 3857c8e and it not only adds more complexity, but it makes your queries slower because you have to do a lot of memory reallocations, so there is more load on the garbage collector.

https://github.com/jdaniel1987/GameShop.CQRS/blob/47adfafb92ef125560ff3e1d7064d12938589342/GameShop.Application/Extensions/GameConsoleExtensions.cs#L57-L77

You are doing three mappings: GameConsole -> GetAllGameConsolesQueryResponse -> GetAllGameConsolesResponse

Hi Dave Roman,

Thank you for your feedback—you're my first commenter! 😊

You raised two important points:

Complexity: Adding transformations can indeed make the code harder to follow. Performance: The additional transformations do consume resources and impact performance. Regarding the first point, I believe that adhering to CQRS principles requires separating concerns, as follows:

GameConsole - Domain Model GetAllGameConsolesQueryResponse - Application Model GetAllGameConsolesResponse - Presentation Model This separation helps in creating different presentation models from a single application model.

For the second point, you are correct that performance can be affected. If possible, could you suggest an approach that maintains this separation of concerns while improving performance?

Thank you again for your input!

MrDave1999 commented 2 months ago

CQRS is not about having 3 models such as domain, application and presentation. CQRS is simple: separate your read and write operations with their respective models.

In other words, the “GetAllGameConsolesQueryResponse” model is more than enough to implement CQRS. You can remove “GetAllGameConsolesResponse” if you really don't see any benefit. In this project CQRS is implemented and only one read and write model is used, which are basically DTOs.

jdaniel1987 commented 2 months ago

Well, actually the best practices in CQRS recommend having a request/DTO object for the presentation layer, a Command/Query object in the application layer (when using MediaTr), and a Domain object for business logic. That's why I thought it should follow the same flow in reverse: Domain -> Application -> Presentation.

I will review it, check other repositories again, and readapt where necessary.

Another thing is that I might be applying CQS instead of CQRS — they are similar but have important differences.

I will fix it bit by bit. Thanks for your comments!

MrDave1999 commented 2 months ago

You're welcome. I think there is nothing more to say. I close this issue.