vlingo / xoom-common

The VLINGO XOOM platform SDK common tools shared across various projects.
Other
16 stars 10 forks source link

Adds Outcome monad transformer for Completes #26

Closed buritos closed 4 years ago

VaughnVernon commented 4 years ago

@buritos I like it, and I think it solves the problem for @wwerner but I will wait for @kmruiz to take a look. Since Wolfgang needs it in the morning if Kevin doesn't have time to review please go ahead and merge for Wolfgang. We can "review it in the wild." :)

kmruiz commented 4 years ago

Without having too much context on the reason behind implementing the transformer, I have a few questions and suggestions :D.

With the current implementation, using the transformer is a design decision, and I feel it should be more an implementation detail.

For me this can be refactored later, but I would like to know more on the first question.

Thanks!

VaughnVernon commented 4 years ago

@buritos @kmruiz Thanks! I have merged. You can can continue the discussion regarding Kevin's questions.

buritos commented 4 years ago

Thank you for your review, Kevin.

resolve looks much better, indeed. I've also changed the andThenTo(null) (a hack to get types to align) in the failure branch to Failure.of(cause). Funny how my eyeballs were sticking out when I saw that again now, considering that it was just yesterday I wrote this code :)

To your questions and comments.

Should the CompletesOutcomeT implement the whole contract of Completes?

I don't think that it should implement the Completes interface (as in implements Completes). Perhaps we could implement convenience methods down the road as needed, driven by use cases and feedback. You are of-course welcome to add features at will if you need more stuff in there. Unfortunately, as far as I know, please correct me if I am wrong, because of generics in Java, we can't segregate the Completes interface into Functor, Monad, etc., so that a single transformer OutcomeT could work with any implementation of those that have an Outcome in it. A new class must be created for every combination.

With the current implementation, using the transformer is a design decision, and I feel it should be more an implementation detail.

I am not sure I understand what you mean by that. Without the transformer, one would have to reinvent this every time one wants to compose functions with signatures like the addOne, addTwo, etc. from the test that I included. Given that Completes is part of the public API as the return type of actor protocol methods, and Outcome is what Vlingo offers for dealing with expected domain errors (kudos for that 👍), I would expect that this will be common. It seems to be common in schemata, and for a good reason. See our conversation for the alternatives we explored in the actors channel on Slack.

I would love to understand what you mean by design decision vs implementation detail, so you are most welcome to summon a google meet during daytime (CET) if you have the time for it.

Thanks, Kevin!