-
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
A version of compile that returns a side value #201
Comments
I'd really like to have this, but am stuck in the age old problem of naming. I think changing I don't like using a prime, as I associate that was strictness. Any preferences for names? |
Cc @HeinrichApfelmus and @mitchellwrosen |
Hmm... no strong feelings either way 😅. In defense of the status quo, I've only ever called Having Overall, it's unclear to me whether it'd be better to have:
var <- newEmptyMVar
network <- compile (theNetwork >> liftIO (putMVar var thing))
thing <- readMVar var
actuate network
network <- compile theNetwork
actuate network would instead look like ((), network) <- compile theNetwork
actuate network
Reflecting on it a bit... I think I weakly prefer (1), followed by (2), then (3) |
The reason I'm interested in this is because it simplifies the following pattern: setup :: ... -> IO Callbacks
setup ... = do
(aAddHandler, fireA) <- newAddHandler
(bAddHandler, fireB) <- newAddHandler
...
(zAddHandler, fireZ) <- newAddHandler
actuate =<< compile do
onA <- fromAddHandler
onB <- fromAddHandler
...
onZ <- fromAddHandler
...
return Callbacks{ fireA, fireB, ..., fireZ } to setup :: ... -> IO Callbacks
setup ... = do
(eventNetwork, callbacks) <- compile do
(onA, fireA) <- newEvent
(onB, fireB) <- newEvent
...
(onZ, fireZ) <- newEvent
...
return Callbacks{..}
actuate eventNetwork
return callbacks Smuggling that out with an I'm also fairly liberal with breakage (and don't really think @HeinrichApfelmus Maybe you want to be a tie-breaker 😄 |
Good point, I like that pattern. Ok, I switch to (2), then (3), then (1). |
I wasn't sure how other people would feel about it, but if it's on the table option 2 is my personal favorite. |
As far as the aesthetics of the interface goes, I would prefer (2) and (3) as well. However, there is catch, and this catch is the main reason why I made it difficult to get stuff out of a The catch is that it now also becomes possible to return setup :: … -> IO ()
setup … = do
(networkA, eA) <- compile' $ do
(e, fire) <- newEvent
…
return e
(networkB, _) <- compile' $ do
(eB, fireB) <- newEvent
let eBoom = unionWith (+) eA eB -- 😱
reactimate $ print <$> eBoom -- 😱😱😱
return ()
actuate networkB
actuate networkA In earlier versions of reactive-banana, I actually used a type parameter to prevent this from happening, but eventually decided that this parameter made so much noise in the types that we would be better of with disallowing return values in So, unfortunately, I'd like to stick to (1). (But happy to add an explanation of this somewhere in the source code). |
Honestly, I think this is overly conservative. We could put network ids in |
You could've just removed the "Frameworks t" constraint to get rid of most of the noise. The ST monad is the canonical example of using a type parameter to prevent leaks between different invocations; it doesn't use a type constraint. |
I agree with @ocharles, but I get where @HeinrichApfelmus is coming from. I think the pattern of "don't return this value here, which we do not attempt to prevent with the type system" is common (e.g. whenever acquiring a resource that's only usable before it's released/finalized). And @ChristopherKing42, the "ST monad trick" is indeed how the old API used to work, and is precisely what "so much noise in the types" refers to - not the one |
Right, that makes sense
Oh, sorry about that! Yeah, I was a bit rude in retrospect, sorry. |
Should this issue be closed? If I understand correctly, I'd summarize the discussion as:
|
I think it would be useful if there was a version of
compile
that in addition to compiling the network allowedMomentIO
to return a side value:This allows you to, for example, extract the initial values of behaviors. Looking at the source code, it appears that there already is code to do the above, it just isn't exported in a nice way.
The text was updated successfully, but these errors were encountered: