Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow exception to propagate from fold Success function #736

Merged
merged 1 commit into from
Mar 21, 2018

Conversation

stoyle
Copy link
Contributor

@stoyle stoyle commented Mar 21, 2018

Handling a failed Success as if it was the same as original Failure is not the way it is done in cats. I would argue this makes for a better fold function.

In spirit of cats, don't handle exceptions in a Try.fold.
@stoyle
Copy link
Contributor Author

stoyle commented Mar 21, 2018

Hi guys! I guess you've made a really explicit choice to include this behavior. I hope you will consider changing it, it really confused me in our code base.

Cheers,
Alf

@pakoito
Copy link
Member

pakoito commented Mar 21, 2018

I believe it was me who made the change. I don't feel strongly for or against it. The version in this PR is more correct specially coming from Scala, but less user-friendly for the Java people :D

@jasonab
Copy link
Contributor

jasonab commented Mar 21, 2018

As a Java person, I agree with the change - it would be a real headache to track down that it was the success function that was causing the failure, not the previous function!

@pakoito pakoito merged commit 71ec421 into arrow-kt:master Mar 21, 2018
@pakoito
Copy link
Member

pakoito commented Mar 21, 2018

Merged it is!

@stoyle
Copy link
Contributor Author

stoyle commented Mar 22, 2018

That was quick, awesome, thanks :)

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.

None yet

3 participants