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

MonadReader instance for Codensity #10

Closed
jonsterling opened this issue Nov 13, 2014 · 3 comments · Fixed by #12
Closed

MonadReader instance for Codensity #10

jonsterling opened this issue Nov 13, 2014 · 3 comments · Fixed by #12

Comments

@jonsterling
Copy link
Contributor

Is there a particular reason that Codensity does not implement MonadReader? If not, I'd be happy to submit a patch. Thanks!

@ekmett
Copy link
Owner

ekmett commented Dec 12, 2014

I'm somewhat torn.

On one hand, yes, it can be implemented.

On the other hand, it is effectively saying "hey I have a state, which is large enough to implement the MonadReader API as well".

It seems analogous to me to wanting MonadReader s (State s) or Monoid s => MonadWriter s (State s) which would be fine if we had more of a sub-typing style of relationship between State and Reader (with ask/get being unified) or State and Writer (with put/tell being unified), but we culturally haven't chosen to embrace that notion.

@jonsterling
Copy link
Contributor Author

@ekmett that's a fair point. The main reason I meant to add MonadReader to Codensity was to enable the composition of code which already uses MonadReader into a Codensity computation, without having to either lift or write a conversion.

In any case, the solution in the absence of this instance is trivial enough that I'm not going to argue strongly for it, though it seems like a nice thing to have.

@ekmett
Copy link
Owner

ekmett commented Dec 13, 2014

Merged. I'd rather have the instance than not, and it's not like there is a viable alternative instance.

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 a pull request may close this issue.

2 participants