Skip to content

Add Result.tap for side effects without altering the result#255

Open
jfvillablanca wants to merge 1 commit intolune-climate:masterfrom
jfvillablanca:master
Open

Add Result.tap for side effects without altering the result#255
jfvillablanca wants to merge 1 commit intolune-climate:masterfrom
jfvillablanca:master

Conversation

@jfvillablanca
Copy link
Copy Markdown

Add a tap method to Result that calls a mapper function on Ok values for side effects but returns the original value. If the mapper returns an Err, that error is propagated. This is similar to andThen but discards the mapper's success value.

Inspired by Effect's tap and RxJS's tap operators.

#254

Add a tap method to Result that calls a mapper function on Ok values
for side effects but returns the original value. If the mapper returns
an Err, that error is propagated. This is similar to andThen but
discards the mapper's success value.

Inspired by Effect's tap and RxJS's tap operators.
@jfvillablanca
Copy link
Copy Markdown
Author

I split up the test cases into multiple test cases, I noticed that there are no describe blocks and most tests rely on comments or implicit understanding of the tests. let me know if I need to rewrite my tests for uniformity.

Copy link
Copy Markdown
Collaborator

@jstasiak jstasiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thank you for this.

At the moment I'm pretty convinced the new method should make no use of the value returned by the callbacks, possibly even only accept callbacks returning void to make it less error-prone.

Otherwise as-is it kind of duplicates andThen and in a non-obvious way (because it discards success values but forwards error values).

Is this an intentional design decision?

I split up the test cases into multiple test cases, I noticed that there are no describe blocks and most tests rely on comments or implicit understanding of the tests. let me know if I need to rewrite my tests for uniformity.

I think this is fine, don't worry about it.

PS. When the design is agreed on we also need documentation in docs/ (exactly matching the JSDoc documentation).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants