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

Implement initial BindLeft and MonadLeft #17

Closed
wants to merge 24 commits into from

Conversation

JordanMartinez
Copy link
Contributor

@JordanMartinez JordanMartinez commented Nov 15, 2020

Fixes #12

WIP

Things to do:

  • Implement qualified do for ldiscard, lbind, lmap, and lapply

-- | import Control.MonadLeft.Qualified as MonadLeft
-- |
-- | foo :: Either Int String -> Either String String
-- | foo comp = MonadLeft.ado
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should the convention be Flip.do rather than MonadLeft.do?

Copy link
Member

Choose a reason for hiding this comment

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

MonadLeft.do seems more meaningful to me than Flip.do, but writing MonadLeft.ado for a type with only an ApplicativeLeft instance and no MonadLeft is odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I read MonadLeft.do / MonadLeft.ado less of a "hey, this thing is a monad!" and more of a "hey, we're using the MonadLeft type class hierarchy here." That's how I read Ix.do, React.do, and Hooks.do.

@JordanMartinez
Copy link
Contributor Author

What advantage would there be for defining an instance of MonadLeft and its superclasses instances for Flip if we have qualified do?

Copy link
Member

@kl0tl kl0tl left a comment

Choose a reason for hiding this comment

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

While lapply, lpure, lbind and ldiscard are consistent with lmap, llift* is awkward and lifM reads like a typo. This is consistent with the prefixes from bifunctors and indexed-monad though, so perhaps this ship has already sailed.

src/Control/ApplicativeLeft.purs Outdated Show resolved Hide resolved
src/Control/ApplyLeft.purs Outdated Show resolved Hide resolved
src/Control/BindLeft.purs Outdated Show resolved Hide resolved
src/Control/BindLeft.purs Outdated Show resolved Hide resolved
src/Control/BindLeft.purs Outdated Show resolved Hide resolved
-- | import Control.MonadLeft.Qualified as MonadLeft
-- |
-- | foo :: Either Int String -> Either String String
-- | foo comp = MonadLeft.ado
Copy link
Member

Choose a reason for hiding this comment

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

MonadLeft.do seems more meaningful to me than Flip.do, but writing MonadLeft.ado for a type with only an ApplicativeLeft instance and no MonadLeft is odd.

src/Control/MonadLeft/Qualified.purs Show resolved Hide resolved
@JordanMartinez
Copy link
Contributor Author

This is consistent with the prefixes from bifunctors and indexed-monad though, so perhaps this ship has already sailed.

We could always break convention here now and update bifunctors and indexed-monad to use a *Left suffix in a future breaking changes release. In this case, I'd rather have something readable than something short.

@JordanMartinez
Copy link
Contributor Author

CI now builds.

@JordanMartinez
Copy link
Contributor Author

The only thing blocking this PR is deciding on the naming convention for referencing this qualified do. I think BiLeft should be used as it's short for bifunctor and the left parameter used in the bifunctor.

@JordanMartinez
Copy link
Contributor Author

I moved all the modules implemented here into a Control.Bifunctor module. This is so we reduce possible breaking changes if we ever want to implement a MonadLeft-like type class hierarchy for trifunctors.

I've also updated the naming convention for the qualified do to use BiLeft which distinguishes it from the Left constructor of Either and ties it to the underlying bifunctor type's left type parameter.

Can I get an approval on this?

@hdgarrood
Copy link
Contributor

This is so we reduce possible breaking changes if we ever want to implement a MonadLeft-like type class hierarchy for trifunctors.

This feels like a case of YAGNI to me. It's hard for me to see us ever wanting to do that in core; even this version of MonadLeft has only generated minimal interest.

@JordanMartinez
Copy link
Contributor Author

It's hard for me to see us ever wanting to do that in core; even this version of MonadLeft has only generated minimal interest.

How about I publish this as a separate library then?

@hdgarrood
Copy link
Contributor

I don't have a strong view on whether MonadLeft should be in core or not, but I think the argument that it should be here so that we can provide Monad Flip makes sense to me. It's just that I think it probably is safe to say that a version of MonadLeft which works on the first of three type variables is not useful often enough to deserve to be in core, and therefore I'd suggest not designing this library in such a way as to make space to do that in the future.

@JordanMartinez
Copy link
Contributor Author

Ah... That does make sense.

@JordanMartinez
Copy link
Contributor Author

Closing as this hasn't received an approval or further feedback.

@JordanMartinez JordanMartinez deleted the addBindLeft branch March 4, 2023 03:33
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.

Add a BindLeft type class
3 participants