search

serokell / universum

:milky_way: Prelude written in @Serokell
MIT License
176 stars 28 forks source link

ExceptT reexport is confusing in absence of MonadError #198

Open JustusAdam opened 6 years ago

JustusAdam commented 6 years ago

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 commented 6 years ago

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.

volhovm commented 6 years ago

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.