rom icon indicating copy to clipboard operation
rom copied to clipboard

Command/Query Separation for ROM::Commands

Open Krule opened this issue 6 years ago • 7 comments

According to CQS principle:

Every method should either be a command that performs an action, or a query that returns data to the caller, but not both.

With that in mind I would like to see a refactor of the following with at least an option for a commands not to perform additional database query in order to return the updated state (especially since we usually know what state is expected to be a result of the execution in advance):

  • https://github.com/rom-rb/rom/blob/master/lib/rom/commands/create.rb
  • https://github.com/rom-rb/rom/blob/master/lib/rom/commands/update.rb
  • https://github.com/rom-rb/rom/blob/master/lib/rom/commands/delete.rb

Krule avatar Oct 24 '17 14:10 Krule

@solnic you were saying that you regret adding result: :many or result: :one but maybe a custom command set with result: :void could trigger this behavior?

abrthel avatar Nov 02 '17 01:11 abrthel

@abrthel oh we definitely do not want to add another option for result :) I think all that's needed, is splitting methods in a way that we don't get results back in the same method. Once it's separated, we can conditionally get results using a separate method, and it could be configured.

solnic avatar Nov 02 '17 02:11 solnic

@solnic how would you envision configuring if results are returned or not?

I imagine, you'd want to be able to do something like: relation.command(:create, option_name: :value)

but what would you call the option?

abrthel avatar Nov 02 '17 03:11 abrthel

So, been thinking about this and realized it's something we want to do in rom 5.0 / rom-sql 3.0.

solnic avatar Nov 17 '17 09:11 solnic

@solnic Did something to fix this behavior make it into rom 5.0 / rom-sql 3.0? I'd like to avoid an additional SELECT query after a :create command.

paddor avatar May 19 '20 15:05 paddor

@paddor no, but it's very likely it'll be addressed in rom 6.0.0 and rom-sql 4.0

solnic avatar May 20 '20 09:05 solnic

I allowed myself to move this issue to the rom repository, because it's going to be a core feature and it will Just Work™ in rom-sql. This is now scheduled for 6.0.0 release.

solnic avatar Jun 25 '20 08:06 solnic