On Tue, Nov 16, 2010 at 10:11 PM, Amy de Buitléir
Patrick LeBoutillier
writes: Amy,
Here's a small suggestion:
If the start and loop functions always returns Nothing, I think it may be cleaner to simply make them return ().
Thank you Patrick, that make the interface easier for the user to understand.
The "loop" function does return something when it is called recursively, but the outer call never returns anything, so my first try was to have two separate functions.
Even recursively it doesn't return anything, or rather it always ends up returning Nothing since that's what the base case returns and the recursive case don't modify it, also finalise already returns (), so you can write :
loop :: (a -> IO a) -> (a -> IO ()) -> a -> IO () loop work finalise d = do timeToStop <- readMVar termReceived if timeToStop then finalise d else work d >>= loop work finalise
To avoid the need to repeat the same parameters at each call of the recursive function, you can use a "worker-wrapper" (? not sure of the term) transformation :
loop :: (a -> IO a) -> (a -> IO ()) -> a -> IO () loop work finalise d = loop' d where loop' d = do timeToStop <- readMVar termReceived if timeToStop then finalise d else work d >>= loop'
Which finally can be directly included in your start function :
start -- | This function will be invoked when the daemon starts. :: IO a -- | This function will be invoked in the main loop. -> (a -> IO a) -- | This function will be invoked when the daemon shuts down. -> (a -> IO ()) -- | Returns nothing. -> IO () start initialise work finalise = installHandler sigTERM (Catch handleTERM) Nothing >> initialise >>= loop where loop d = do timeToStop <- readMVar termReceived if timeToStop then finalise d else work d >>= loop
Defining functions at the top level instead may be clearer or more useful in certain cases, but I'm not sure this is one of those cases (you don't want to export loop, start and loop bodies are very short, and you can simplify and optimize loop body by making it a local function). -- Jedaï