-
Notifications
You must be signed in to change notification settings - Fork 71
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
Generalise newAddHandler
to any MonadIO m
#225
base: master
Are you sure you want to change the base?
Conversation
This makes it slightly more convenient to call when you're in other monads (for example, `RIO`), saving the user a `liftIO`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM; I tend to like to {-# specialize #-} IO versions of such things but it probably reeeally doesn't matter here :)
Does that really do anything? The |
Right; I think it does do something, namely, it generates code that avoids passing that dictionary at runtime. |
Hm. Alas, I'm not entirely sold on the introduction of Do you have a specific example where avoiding the (I suppose that exception hierarchies à la That said, I would consider making the code polymorphic with the help of the io-classes package — in this case, the io-sim package would give us the ability to run the code in a simulated |
It's certainly more complicated, but is it more complicated beyond the expected experience for users of this library? I'm not sure!
I do find this a little contradictory - we don't want to to
Saves the day is a little strong, I will probably never have such an example as all this change does is allow me to drop a data APIEnv = APIEnv
{ getRecentMessages :: MVar (Seq LogMessage) -> IO ()
, doAOIPocket :: AOIRequest (Text, Int32) -> IO ()
, -- lots more `a -> IO ()` "fire" functions
}
data AddHandlers = AddHandlers
{ onUpCameraImageAH :: AddHandler Image
, onDownCameraImageAH :: AddHandler Image
, -- lots more `AddHandler`s
}
main :: IO ()
main = do
-- not interesting
...
withBatchedHandler defaultBatchingOptions emitJSONMessages \logHandler ->
flip runLoggingT (liftIO . logHandler) $
evalContT do
-- Open both the up and down cameras
pylon <- ContT withPylon
onDownCameraImageAH <- openCamera CameraConfig
{ serialNumber = "40123497"
, imagesDirectory = "/var/lib/pnp/down-camera"
}
onUpCameraImageAH <- openCamera CameraConfig
{ serialNumber = "40100047"
, imagesDirectory = "/var/lib/pnp/up-camera"
}
(onSampleStateAH , getImageDb ) <- liftIO newAddHandler
(onSampleMessagesAH, getRecentMessages) <- liftIO newAddHandler
-- Start part centering
adsConnection <- ContT $ withConnection $ adsConnectInfo plcHost
twinCatVariables <- ContT $ withTwinCatVariables adsConnection
-- Create AddHandlers for API calls
(onDoFidFindAH , doFidFind ) <- liftIO newAddHandler
(onLoadProgramAH , doLoadProgram ) <- liftIO newAddHandler
(onAOIReferenceDesignatorAH, doAOIReferenceDesignator) <- liftIO newAddHandler
(onAOIPointAH , doAOIPoint ) <- liftIO newAddHandler
(onResetAH , doReset ) <- liftIO newAddHandler
(onLoadPalletAH , doLoadPallet ) <- liftIO newAddHandler
(onAOIFiducialsAH , doAOIFiducials ) <- liftIO newAddHandler
(onPickAndPlaceAH , doPickAndPlace ) <- liftIO newAddHandler
(onAOIPocketAH , doAOIPocket ) <- liftIO newAddHandler
liftIO $ actuate =<< compile do
onLogMessages <- mainFRP twinCatVariables AddHandlers{..}
reactimate $ traverse_ logHandler <$> onLogMessages
serveAPI APIEnv{ doReset = doReset (), ..} Note that there's a significant amount of |
Hm, I see. 🤔 The Would you open to a compromise where a new module
My thinking was that if we go into this direction at all, then we might as well go all the way. I tend to think of
The beauty (and purpose) of this approach is that the monad does not need to be a stack of monad transformers whose ground type is IO — instead, the ground type can be Identity or any pure data type. This would give us the ability to run/simulate the entire network in a pure manner, completely without IO. In contrast, the By the way, your records that contain import GHC.Generics ((:*:) (..))
data Handlers f = Handlers
{ upCameraImage :: f Image
, downCameraImage :: f Image
, …
}
type APIEnv = Handlers Handler
type AddHandlers = Handlers AddHandler
hoist :: (forall a. f a -> g a) -> Handlers f -> Handlers g
hoist f (Handlers a b c d) = Handlers (f a) (f b) (f c) (f d)
unzipHandlers :: Handlers (f :*: g) -> (Handlers f, Handlers g)
unzipHandlers h = (hoist first h, hoist second h)
where
first (x :*: _) = x
second (_ :*: y) = y
newAddHandler' :: MonadIO m => m ((AddHandler :*: Handler) a)
newAddHandler' = liftIO $ fmap to newAddHandler
where to (x,y) = x :*: y
main :: IO ()
main = do
…
(onHandlersAH, doHandlers) <- fmap unzipHandlers $ Handlers
<$> newAddHandler'
<*> newAddHandler'
<*> newAddHandler'
<*> newAddHandler' In this way, you would not have to write down two distinct names for each pair of
Ooh! That's a very neat trick. I'm looking at a similar problem right now (long chains of |
I'm not really convinced. My proposed change to Minor, but it also increases the dependency graph if you really want
It feels like this would be quite a lot of work, and moves
Personally, I think we can all get a bit too caught up in this goal - I know I have. In reality, being in
Yep, I'm aware of this pattern, but generally like to write the simplest Haskell I can. In this case, I couldn't quite justify this pattern. (Somewhere I think it is justified is in my We're going a bit off topic now though. I think we should wrap up and make a decision. I think there are ultimately two choices:
I'm just not sure how to make the choice! Personally I much prefer I can't find much motivation for 2 - it just makes things slightly more annoying. If you're in |
I think (1) has small cost and small benefit. One thing I dislike is how there's not much that is generalizable to |
Just for reference: The types
My reason for bringing up the My thinking was that I have a slight preference for the monomorphic version (choice 2), but that we could have the best of both worlds by also offering choice 1 in the "de facto canonical" namespace for those who want it.
Fair enough. I ditch the |
This makes it slightly more convenient to call when you're in other
monads (for example,
RIO
), saving the user aliftIO
.