Introduce HasSource, and make superclasses #75
Conversation
aspiwack
left a comment
There was a problem hiding this comment.
I browsed this quickly. I rather like the idea of Sink and Source.
Thanks for this. And let us know when this is ready for careful review.
88a8d3c to
9cde9c1
Compare
|
Have at it! |
aherrmann
left a comment
There was a problem hiding this comment.
I really like this PR, thank you! Just a few small comments, then I'm happy to merge it.
| CountLog | ||
| Error | ||
| Reader | ||
| Sink |
There was a problem hiding this comment.
Do you happen to have a Source only example?
There was a problem hiding this comment.
I should make one.
| StateOverStream (StateT Int (Stream (Of (Int, Char)) IO) a) | ||
| deriving (Functor, Applicative, Monad) | ||
| deriving (HasState "counter" Int) via | ||
| deriving (HasSource "counter" Int, HasSink "counter" Int, HasState "counter" Int) via |
There was a problem hiding this comment.
Outside the scope of this PR, but I wonder if there's a way to avoid this boilerplate of explicitly deriving all superclasses. From a user's perspective that's a cost of the finer grained class hierarchy. And it means that adding a new superclass will often break user's code.
Maybe that's really an issue with deriving-via. cc @aspiwack
This may be a terrible idea, but just to throw it out. For the user facing capabilities we could group all superclasses together into a synonym (or type family) like so:
class (HasSourceClass tag a, HasSinkClass tag a) => HasStateClass tag a where ...
type HasState tag a = (HasSourceClass tag a, HasSinkClass tag a, HasStateClass tag a)
There was a problem hiding this comment.
I don't think this packing of classes would work: you can only derive type classes, not conjunctions of them.
And, generally, yes: adding subclasses breaks code. It is true that, in this case, it means that every derivation of State need to be somewhat triplicated. Which is not very nice.
There was a problem hiding this comment.
Yeah this is a downside I didn't know how to fix. I'm not sure about the ramifications of super and subclasses from different newtypes with derive-via; Whether that is possible affects whether making one list them all adds any information.
We now have:
```
Source Sink
/ \ / \
/ \ / \
Reader State Writer
```
HasStream has been renamed (with deprecated aliases for compatibility) to HasSink, to
illustrate the symmetry.
Co-Authored-By: Andreas Herrmann <andreash87@gmx.ch>
be93757 to
e9e121b
Compare
|
@Ericson2314 Thanks for implementing this! I'm merging now, sorry for the delay. A |
|
Oh thank you! Sorry I didn't get it to you already, I had this tab open as a TODO. I'll keep this tab open as a TODO. |
We now have:
HasStream has been renamed (with deprecated aliases for compat) to HasSink, to
illustrate the symmetry.
WIP because I need to review myself, and probably also fix examples.
Fixes #63.