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.

Sign up for GitHub

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

Jump to bottom

Consider switching Option.match parameter order? #52

Closed
marroen opened this issue Sep 29, 2022 · 1 comment · Fixed by #56
Closed

Consider switching Option.match parameter order? #52

marroen opened this issue Sep 29, 2022 · 1 comment · Fixed by #56
Assignees
@SandroMaglione
Labels
chore Rename files, upgrade dependencies, etc. (job often boring but needs to be done regularly) refactoring No change in functionality, but rewrite of some code

Comments

@marroen
Copy link

marroen commented Sep 29, 2022

This is only a matter of personal preference, but here's my reasoning:
You routinely handle Eithers first with onLeft, then onRight. In other words, you handle the error first, which is fine.
For Options however, you handle the Some value before the None value. In other words, you handle the "error" last.

I understand this would mean a lot of refactoring for a lot of people, but I think this is more intuitive on the long-term.
@SandroMaglione SandroMaglione self-assigned this Oct 7, 2022
@SandroMaglione SandroMaglione added the chore Rename files, upgrade dependencies, etc. (job often boring but needs to be done regularly) label Oct 7, 2022
SandroMaglione added a commit that referenced this issue Oct 7, 2022
@SandroMaglione
Invert onNone/onSome in Option match
2f31b8d 
fixes #52
@SandroMaglione SandroMaglione mentioned this issue Oct 7, 2022
Merged
@SandroMaglione SandroMaglione linked a pull request Oct 7, 2022 that will close this issue
Option match invert onNone and onSome #56
Merged
@SandroMaglione SandroMaglione added the refactoring No change in functionality, but rewrite of some code label Oct 7, 2022
@RandalSchwartz
Copy link
Contributor

Even though this is a breaking change, it can't fail silently, and that's a good thing.

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

@SandroMaglione SandroMaglione

Labels
chore Rename files, upgrade dependencies, etc. (job often boring but needs to be done regularly) refactoring No change in functionality, but rewrite of some code
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Option match invert onNone and onSome SandroMaglione/fpdart
3 participants
@RandalSchwartz @SandroMaglione @marroen