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

ExceptT reexport is confusing in absence of MonadError #198

Open
JustusAdam opened this issue Oct 3, 2018 · 2 comments
Open

ExceptT reexport is confusing in absence of MonadError #198

JustusAdam opened this issue Oct 3, 2018 · 2 comments
Labels
type:breaking Breaking change (removal, renaming, semantic change, etc.)

Comments

@JustusAdam
Copy link
Contributor

I think the absence of MonadError and the reexport of ExceptT is a confusing choice.

As rightfully mentioned in #13 Universum already includes MonadThrow and MonadCatch, however these interact with ExceptT in ... possibly confusing ways.

The principle problem here is that MonadThrow and MonadCatch are supposed to catch actual Exceptions, not the user defined errors that are used in ExceptT. While I understand and agree with this decision it may be strange for newer users of MonadThrow.

Specifically with the omission of MonadError there is no (sensible) way to interact with a stack containing ExceptT and throw an error into ExceptT or catch it.

My suggestion is to remove ExceptT from the library entirely as it is not particularly useful anyways when actual exceptions are available.

Alternatively I would suggest reversing the decision from #13 and reintroduce MonadError to allow users to interact with ExceptT.

@gromakovsky
Copy link
Member

Hm, strange, I am reading discussion in #13 and see this:

Also I will remove Control.Monad.Except module if no one objects. Seems like nobody uses ExceptT, runExceptT and other stuff.

Apparently it was intended to remove ExceptT, but somehow it survived.
I don't want to browse history to understand whether it was removed and then re-added or it wasn't removed at all. My latest experience with ExceptT is that it can sometimes be useful, but sometimes its usage may even be harmful and it's not something that should be exported from a custom prelude.

So I approve removal of ExceptT and runExceptT, but it's a breaking change and we need more opinions on that.

@gromakovsky gromakovsky added the type:breaking Breaking change (removal, renaming, semantic change, etc.) label Oct 4, 2018
@volhovm
Copy link
Contributor

volhovm commented Oct 14, 2018

I find ExceptT to be generally very useful in different situations -- the most popular use case is just wrapping some specific function in the runExceptT call so that it's possible to handle multiple local exit points in a handy way. I'm mostly against wrapped/nested ExceptT and using MonadError in general, so the ExceptT with the runner function is enough I think. Actually, I'd like to have the throwError also, but it'd be better to have it specialised to the ExceptT exclusively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:breaking Breaking change (removal, renaming, semantic change, etc.)
Projects
None yet
Development

No branches or pull requests

3 participants