Create a gist now

Instantly share code, notes, and snippets.

I've got that piece of code that looks extremely overcomplicated to me. Even though every operation by itself is simple, the "sum" of operations is simply insane.
data Query = Query
data SomeObj = SomeObj
data IoOnlyObj = IoOnlyObj
data Err = Err
-- There's a decoder function that makes some object from String
decodeFn :: String -> Either Err SomeObj
decodeFn = undefined
-- There's a query, that runs against DB and returns array of strings
fetchFn :: Query -> IO [String]
fetchFn = undefined
-- there's some additional "context initializer", that also has IO
-- side-effects
makeIoOnlyObj :: [SomeObj] -> IO [(SomeObj, IoOnlyObj)]
makeIoOnlyObj = undefined
-- and now, there's a pipeline function, that takes query,
-- decodes results, and then runs another IO operation with the
-- results of query. And it seems to me that there are much much
-- better ways of implementing such things.
--
-- `makeIoOnlyObj` returns IO-wrapped result, and we need
-- return IO Either-wrapped.
--
-- So far what I've got is as follows. How do I improve it?
pipelineFn :: Query
-> IO (Either Err [(SomeObj, IoOnlyObj)])
pipelineFn query = do
a <- fetchFn query
case sequence (map decodeFn a) of
(Left err) -> return $ Left $ err
(Right res) -> do
a <- makeIoOnlyObj res
return $ Right a
@YoEight
YoEight commented Sep 27, 2014

In such use case, monad transformers is what you need. So I gonna use ExceptT monad transformer. It augments any Monad m to support failure.

You can find it in transformers package

import Control.Monad.Trans.Except
import Data.Traversable (traverse)

data Query     = Query
data SomeObj   = SomeObj
data IoOnlyObj = IoOnlyObj
data Err       = Err

-- use `throwE` to introduce failure case
decodeFn :: Monad m => String -> ExceptT Err m SomeObj
decodeFn = undefined

fetchFn :: Query -> IO [String]
fetchFn = undefined

makeIoOnlyObj :: [SomeObj] -> IO [(SomeObj, IoOnlyObj)]
makeIoOnlyObj = undefined

pipelineFn :: Query -> ExceptT Err IO [(SomeObj, IoOnlyObj)]
pipelineFn query = do
  as   <- liftIO $ fetchFn query
  as'  <- traverse decodeFn as
  liftIO $ makIoOnlyObj as'

At some point, you'll need to evaluate your ExceptT computation. All you have to do is using runExcept function.

runExceptT :: ExceptT e m a -> m (Either e a)
@lucasdicioccio

To complement YoEight's answer, here is a solution which does the same as yours, but shorter. This version of your code saves a few lines by injecting the makeIoOnlyObj and the decoding inside the query result using Functor properties of IO and Either Err.

pipelineFn = do
   -- (a) turns the query into an error/an action
   x <- (fmap makeIoOnlyObj . traverse decodeFn) <$> fetchFn query
   -- (b) propagates the error or run the action
   either (return . Left) (Right <$>) x

I prefer YoEight's answer but I think this form is useful: in this form, line (b) illustrates why YoEight tells that, in such use case, your problem asks for a transformer. Line (b) tries to pop the action out of the Either Err while keeping the same error/no-error context (i.e., a good old try / catch block), which is the job of the ExcepT. In summary, my solution is a middle ground between the two other solutions, and may help you identify why a transformer is preferrable. If your codebase grows, the ExcepT saves you from typing the either (return . Left) (fmap Right) every time you chain actions (you'll pay in lifts, which are much cheapers because they do not expose internals (unlike 'either')).

@ifesdjeen
Owner

Another variation of traverse version, suggested by @bitemyapp is:

pipelineFn :: Query
              -> IO (Either Err [(SomeObj, IoOnlyObj)])
pipelineFn query = do
  a <- fetchFn query
  traverse makeIoOnlyObj (mapM decodeFn a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment