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

Adding fromThese and toThese functions #158

Open
subttle opened this issue Mar 14, 2021 · 2 comments
Open

Adding fromThese and toThese functions #158

subttle opened this issue Mar 14, 2021 · 2 comments

Comments

@subttle
Copy link

subttle commented Mar 14, 2021

Hi again, I realize you already have a function named toThese unfortunately, so this would be a breaking change, but I do believe what I'm proposing is the proper move all other things considered equal. If you'll notice in similar data types, such as Smash, one has the case-analysis/eliminator smash and then the toSmash and fromSmash functions. Similarly for Wedge, one has the case-analysis/eliminator wedge and then the toWedge and fromWedge. And soon enough there should be the same for Can, they have the case-analysis/eliminator and then once they bring in the These type they could then in theory write the missing toCan and fromCan:

-- Data.Can
toCan  Maybe (These a b)  Can a b
toCan = maybe Non (these One Eno Two)

fromCan  Can a b  Maybe (These a b)
fromCan = can Nothing (Just . This) (Just . That) (curry (Just . uncurry These))

By the way I'm just speculating, I don't speak on behalf of their project, so I hope I'm not giving that impression!
Back on topic, These already has the case-analysis/eliminator of course, and I think it could also use:

-- Data.These
toThese    Either (Either a b) (a, b)  These a b
toThese   = either (either This That) (uncurry These)

fromThese  These a b                   Either (Either a b) (a, b)
fromThese = these (Left . Left) (Left . Right) (curry (Right . uncurry (,)))

Now I am aware that the Either makes it so that these functions could be written in multiple ways but my argument is that the case-analysis has always been done in the order in which the data type is defined (c.f. maybe, bool, either, etc.), so it would make sense to use the above definitions because they are in keeping with this tradition because as you know These is defined:

data These a b = This a | That b | These a b

So it makes sense to have a normal form with the sum first and the product after. At any rate, sorry for the rant; it is okay if you don't like my suggestion I won't be offended if you close the ticket and move on, but I decided to share just in case you are interested, in which case let me know if you would like a pull request.

Thank you for your time and consideration. And thanks again for all your hard work on this excellent package!

@phadej
Copy link
Collaborator

phadej commented Mar 14, 2021

fromThese follows fromMaybe naming convention.

Converting to/from nested Eithers. I don't know. Does that occur in practice?

I mean. I'm not excited by random abstract non-sense. There have to be a practical purpose, an application of an idea.

Also naming: if it's in Data.These, it should be fromEitherEither and toEitherEither (or something, EitherEither is far from excellent either). c.f. Data.Map.fromList. Data.These.fromThese reads weird (as weird as Data.Smash.fromSmash).

@subttle
Copy link
Author

subttle commented May 9, 2021

fromThese follows fromMaybe naming convention.

Hm, I would think something along the lines of toTuple (or toPair or paired), or something involving "default" in the name would be more fitting but I can also see the wisdom behind your current naming convention.

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

No branches or pull requests

2 participants