sanctuary-js / sanctuary

:see_no_evil: Refuge from unsafe JavaScript
https://sanctuary.js.org
MIT License
3.03k stars 94 forks source link

Add `Either` ↔ `Maybe` conversion functions #644

Open futpib opened 4 years ago

futpib commented 4 years ago

This pull request adds the following functions:

maybeToLeft  :: b -> Maybe a -> Either a b
maybeToRight :: a -> Maybe b -> Either a b

leftToMaybe  :: Either a b -> Maybe a
rightToMaybe :: Either a b -> Maybe b
davidchambers commented 4 years ago

What do others think of these (breaking) changes? @Avaq? @Bradcomp? @masaeedu?

masaeedu commented 4 years ago

@davidchambers do we have a swapEither :: Either a b -> Either b a function? This sort of thing might be better handled by a lens library, but absent a good story for those, functions like the one in this PR can be pretty handy.

davidchambers commented 4 years ago

We have swap :: Pair a b -> Pair b a but not swapEither :: Either a b -> Either b a. Could you explain the relationship you see between swapEither, a lens library, and the functions in this pull request? You seem to be suggesting that there are multiple ways to express these ideas, and that a different approach could render these changes unnecessary.

masaeedu commented 4 years ago

@davidchambers Right, there's a number of different alternatives here. FWIW I think going down the road of exhaustively comparing all these alternatives would swamp the PR, and I think the functions in the PR are simple and handy enough to justify addition as is.

To explain the relationship between the functions here and swapEither: if I understand the changes here correctly, they're implementing a "swapped" version of each xyzEither function.

So for example instead of only having maybeToEither :: a -> Maybe b -> Either a b, it adds maybeToLeft :: a -> Maybe b -> Either b a, which could also be seen as a composition of maybeToEither and swapEither.

Similarly, instead of only having fromEither :: b -> Either a b -> b, you additionally have fromLeft :: b -> Either b a -> b, which again can be seen as b => compose(fromEither(b))(swapEither).

There's also wacky stuff you can do with lenses (have an iso between Either a b and Either b a, and compose it with optics relating maybes and eithers), but there's no good story for this at the moment, and it'd be a ways off.

davidchambers commented 4 years ago

Thanks for the clarification, Asad. I understand your point now. :)

Avaq commented 4 years ago

What do others think of these (breaking) changes?

I'm happy with them. My only concern is that in implementing the entire matrix of conversions between a, Maybe a, Either a b, and Either b a, we might be increasing API surface drastically for functions that can be expressed as simple compositions. The current "matrix":

from :arrow_down: \ to :arrow_right: a Maybe a Either a b Either b a
a I Just Left Right
Maybe a fromMaybe I maybeToLeft maybeToRight
Either a b fromLeft leftToMaybe I ~swapEither~
Either b a fromRight rightToMaybe ~swapEither~ I

Instead of leftToMaybe, one could use a composition of Just and fromLeft, for example. Or the other way around, use a composition of leftToMaybe and fromMaybe to derive fromLeft.

My concern exists because the composition model is easily scalable under inclusion of additional types, whereas the above matrix would grow exponentially with every new constructor added.


I've just spent a lot of words explaining this concern, which makes it seem as though I might feel strongly about this, but it's really very minor - just something you might not have considered.

Avaq commented 4 years ago

Going over these changes again, I think they represent an improvement over the status quo. My API surface scaling concern is small, because these functions will always have descriptive names / namespaces.

davidchambers commented 3 years ago

@futpib, are you interested in submitting a small pull request to change S.fromEither as you did here?

futpib commented 3 years ago

I also dropped the commit with new fromEither from this PR. This is not a major change now, but #690 is.

davidchambers commented 3 years ago

Lovely work, @futpib. Thank you for pointing out that this pull request no longer contains breaking changes.

Please update the commit messages and this pull request's title to reflect the fact that this pull request is purely additive now that #664 has been merged. Also, this pull request's description is no longer accurate.

futpib commented 3 years ago

@davidchambers Done.

davidchambers commented 3 years ago

Thank you, @futpib. This pull request is in a very good state. :)

I am not certain that these functions should be added to the library. Here they are in use:

> maybeToRight ('XXX') (Nothing)
Left ("XXX")

> maybeToRight ('XXX') (Just (42))
Right (42)

> rightToMaybe (Left ('XXX'))
Nothing

> rightToMaybe (Right (42))
Just (42)

The transformations above are possible with existing functions:

> maybe (Left ('XXX')) (Right) (Nothing)
Left ("XXX")

> maybe (Left ('XXX')) (Right) (Just (42))
Right (42)

> either (K (Nothing)) (Just) (Left ('XXX'))
Nothing

> either (K (Nothing)) (Just) (Right (42))
Just (42)

To me, the expressions involving maybe and either are at least as clear as those involving the specialized functions.

Note that maybeToRight can return a Left, which makes sense but reveals the trouble with naming things (see also NaN).

Do these functions offer enough value to justify their inclusion? I would like to hear from @masaeedu, @Avaq, and others. :)

Avaq commented 3 years ago

Note that maybeToRight can return a Left, which makes sense but reveals the trouble with naming things (see also NaN).

Should we consider justToRight?