Skip to content

Add few mtl-class instances#28

Merged
phadej merged 1 commit intoscrive:masterfrom
phadej:monad-error
Feb 8, 2017
Merged

Add few mtl-class instances#28
phadej merged 1 commit intoscrive:masterfrom
phadej:monad-error

Conversation

@phadej
Copy link
Copy Markdown
Contributor

@phadej phadej commented Feb 8, 2017

No description provided.

@phadej phadej changed the title Add few mtl-class instance Add few mtl-class instances Feb 8, 2017
Copy link
Copy Markdown
Contributor

@23Skidoo 23Skidoo left a comment

Choose a reason for hiding this comment

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

See review comments.

deriving (Alternative, Applicative, Functor, Monad, MonadBase b, MonadCatch
,MonadIO, MonadMask, MonadPlus, MonadThrow, MonadTrans)

instance MonadError e m => MonadError e (LogT m) where
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Given that we have GND enabled, can't we avoid writing these instances by hand by mentioning them in the derving clause above?

throwError = lift . throwError
catchError m h = LogT $ unLogT m `catchError` (unLogT . h)

instance MonadReader r m => MonadReader r (LogT m) where
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's better to use the default ReaderT instance for MonadReader instead of this one as well. It'd be used to access LoggerEnv and to access the underlying one you'd use lift ask.

Copy link
Copy Markdown
Contributor Author

@phadej phadej Feb 8, 2017

Choose a reason for hiding this comment

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

I'm 👎, it's a bit like writing of MonadReader s (StateT m) where ask = get, correct but weird. I'd rather leave ability to write LogT (ReaderT r m) a for people if they want to do so.

EDIT I add LogT only to add MonadLog to the stack, nothing else.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK. Just add a comment that the MonadReader instance doesn't give you access to LoggerEnv.

@phadej
Copy link
Copy Markdown
Contributor Author

phadej commented Feb 8, 2017

I'm actually surprised that MonadError e, MonadWriter w, MonadState s works with GND, good do know!

@phadej phadej merged commit a978513 into scrive:master Feb 8, 2017
@phadej phadej deleted the monad-error branch February 8, 2017 18:32
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.

2 participants