Support :ok tagged tuple events from aggregate

[commanded]

Support :ok tagged tuple events from aggregate

[xtagon]

I would much prefer a simple {:ok, events} or {:error, reason} vs having four different return types @drozzy

Receivers that still want to match on whether there are zero, one, or multiple can still do that with pattern matching but aren't forced to implement 4 different matches if they don't care how many events were returned (e.g. they just process all or none of them)

[slashdotdash]

I'm proposing that the change would be backwards compatible, so you can still choose to return just an event, or list of events. The {:ok, events} is in addition to what's currently allowed.

Aggregate return value	Events produced
:ok, nil , []	none
event , {:ok, event}	one event
events , {:ok, events}	list of events
{:error, reason}	an error occurred

@astery The with special form can also sometimes be used to declutter such code.

[astery]

Recently, I tried to write tests with {:ok, ..}, {:error}, and change my mind about it. Code become cluttered with :ok tuples. With current manner aggregate and their tests are clearer. Adding :ok tuples brings no real benefit except being "idiomatic", but some kind of unneeded complexity which is harder to read.

So I would prefer not to change anything.

[xtagon]

What would be the benefit of using {:ok, _} tuples? Is there an {:error, reason} case as well?

[slashdotdash]

Using {:ok, events} and {:error, reason} tagged tuples would make it consistent with idiomatic Elixir code. Commanded already supports {:error, reason}.

[astery]

It's controversial. On one side it's bring a need of code change and extra keystrokes. On over side then I see function returning {:error, _}, I expect to see {:ok, _} as success; and if function return struct, I expect it either return nil or throw an error.

I support this.

[drozzy]

I was going to suggest something like {:events, [Event]}, but after thinking about it, the ok/error combination would be more idiomatic (in fact, I was briefly puzzled by the lack of :ok when first learning commanded - since I didn't know how to indicate an error):

• :ok - In the event command was accepted and no events were produced
• {:ok, event} - In the event command was accepted and one event was produced.
• {:ok, events} - In the event command was accepted and one or more events were produced.
• {:error, Reason} - In the event of the command for rejected for a given reason.

I am not a fan of throwing. Let's not resort to that please.

[drozzy]

Alternatively, we could use something more semantic instead of ok and error, like accepted and rejected.

[drozzy]

I'd rather leave it as is @slashdotdash, that looks you're asking for confusion later on.

[Papipo]

Idiomatic in this case just means more verbose, I don't see the point.

[Freyskeyd]

@slashdotdash are we ok to leave the current implmentation as it is?

Or do we need to add those new format?