Remove the Has type-class
#4
Conversation
src/Example/CountLog.hs
Outdated
| import Data.IORef | ||
| import GHC.Generics (Generic) | ||
|
|
||
| import Has | ||
| import Accessors |
There was a problem hiding this comment.
You should just re-export Accessors from the HasReader and HasWriter. Maybe renaming it to Internal.Accessors.
| (Field "countCtx" (MonadReader (ReaderT CountLogCtx m))))) | ||
| -- XXX: This requires @Field@ and @MonadReader@ to have @MonadIO@ instances. | ||
| -- That seems anti-modular - if a user-defined constraint is required, | ||
| -- they may have to add orphan instances for @Field@ and @MonadReader@. |
There was a problem hiding this comment.
This is a very interesting point. I think that it is fair to lift MonadIO through each of our newtype things (since, well, we are providing combinators using it 🙂 ). Maybe a couple of others (like MonadPrim). But there are arbitrarily many classes which may be relevant.
If a library user makes a new combinator MonadFoo which uses an instances of the class MonadFoo, to build a HasState capability from some other capability, somehow, then they may need to lift MonadFoo through each of our newtypes. Which is probably orphan. Hence a tad fragile.
There is probably not a way around this. But probably, at least every class we mention in the library should be systematically derived for all our newtypes.
src/Example/CountLog.hs
Outdated
| @@ -39,7 +38,7 @@ class Monad m => Logger m where | |||
| logStr :: String -> m () | |||
|
|
|||
| -- | Any @HasReader "logger" (String -> IO ())@ can be a @Logger@. | |||
| newtype TheLoggerReader m a = TheLoggerReader (m a) | |||
| newtype TheLoggerReader (m :: * -> *) a = TheLoggerReader (m a) | |||
There was a problem hiding this comment.
I'm curious why the kind annotation was needed.
There was a problem hiding this comment.
That was an interaction between PolyKinds DerivingVia. Without annotation m would have kind k -> * which made derive via fail. However, PolyKinds is no longer required in the example module, so the annotations can be dropped.
There was a problem hiding this comment.
For clarity: I don't mind. I often put such annotation even when they are not necessary. I was just curious as to why it made it into this particular diff.
| -- 'Control.Monad.Reader.Class.MonadReader' instance. | ||
| newtype MonadReader (m :: * -> *) (a :: *) = MonadReader (m a) | ||
| deriving (Functor, Applicative, Monad, MonadIO) | ||
| instance Reader.MonadReader r m => HasReader tag r (MonadReader m) where |
There was a problem hiding this comment.
For every tag? Why not I suppose.
| -- The constraint raises @-Wsimplifiable-class-constraints@. | ||
| -- This could be avoided by instead placing @HasField'@s constraints here. | ||
| -- Unfortunately, it uses non-exported symbols from @generic-lens@. | ||
| ( Generic r', Generic.HasField' field r' r, HasReader tag r' m ) |
There was a problem hiding this comment.
Maybe call the record r, but the type of the field a instead of r', it would be a hint more readable, I think.
There was a problem hiding this comment.
a is already taken in the instance signatures. Maybe v for value?
src/HasState.hs
Outdated
| deriving via (TheMonadState (SState.StateT s' (m :: * -> *))) | ||
| instance (Has tag s s', Monad m) => HasState tag s (SState.StateT s' m) | ||
| stateOnLens :: Lens' s' s -> (s -> (a, s)) -> s' -> (a, s') | ||
| stateOnLens l f s' = let (a, s) = f (view l s') in (a, set l s s') |
There was a problem hiding this comment.
Fun fact: stateOnLens l is l @(a,) .
Ok, it's more than a fun fact: it's a bit more efficient too 🙂
There was a problem hiding this comment.
Thanks! I thought there must be a better way of doing this. :)
HasHasReader/HasStateinstances forReaderT/StateT.HasReader/HasStatefrom mtl'sMonadReader/MonadStateusing the newtypes with the same name.Fieldto zoom in on record fields inHasReader/HasState.