Could i be using a bind/fmap here

Question

loadTexture :: String -> IO (Either String GL.GLuint)
loadTexture filename = do
    p <- PNG.loadPNGFile filename
    oglLoadImg p
    where
        oglLoadImg :: (Either String PNG.PNGImage) -> IO (Either String GL.GLuint)
        oglLoadImg (Left e) = return $ Left e 
        oglLoadImg (Right png) = do
            ... I need todo IO stuff in here

The code above seems really bloated and nasty. What can I do to make it simpler?

score 14 · Accepted Answer · answered Aug 23 '11 at 20:58

You essentially want a combination of the Either e monad and the IO monad. That's what monad transformers are for!

In this case, you can use the ErrorT monad transformer which adds error handling using Either to an underlying monad, in this case IO.

import Control.Monad.Error

loadTexture :: String -> IO (Either String GL.GLuint)
loadTexture filename = runErrorT $ ErrorT (PNG.loadPNGFile filename) >>= oglLoadImg
    where
        oglLoadImg :: PNG.PNGImage -> ErrorT String IO GL.GLuint
        oglLoadImg png = do
            -- [...]

This keeps the old interface, although it would probably be even nicer to use ErrorT for your function as well, and have the call to runErrorT in your main function.

loadTexture :: String -> ErrorT String IO GL.GLuint
loadTexture filename = ErrorT (PNG.loadPNGFile filename) >>= oglLoadImg
    where
        oglLoadImg :: PNG.PNGImage -> ErrorT String IO GL.GLuint
        oglLoadImg png = do
            -- [...]

Monad transformers can take some getting used to, but they are very useful.

Instead of `ErrorT (PNG.loadPNGFile filename)` is it possible to use `return (PNG.loadPNGFile filename)`? — Zhen, Aug 24 '11 at 07:07

score 12 · Answer 2 · answered Aug 23 '11 at 20:59

Before doing stylistic refactoring, let's take a step back and think about the semantics of what your code is doing here.

You've got an IO action that produces something of type Either String PNG.PNGImage, where the Left case is an error message. You think want to do something with the Right case when it exists, while leaving the error message as is. Think about what this composite operation might look like, if you condensed it into a single, generalized combinator:

doIOWithError :: IO (Either String a) -> (a -> IO b) -> IO (Either String b)
doIOWithError x f = do x' <- x
                       case x' of
                           Left err -> return (Left err)
                           Right y  -> f y

While that could be useful as is, you may already have noticed that its type signature looks suspiciously similar to (>>=) :: (Monad m) => m a -> (a -> m b) -> m b. In fact, if we generalize one step further by allowing the function to produce errors as well, we have exactly the type of (>>=) where m a becomes IO (Either String a). Unfortunately, you can't make that a Monad instance, because you can't just glue type constructors together directly.

What you can do is wrap it in a newtype alias, and in fact it turns out that someone already has: this is just Either used as a monad transformer, so we want ErrorT String IO. Rewriting your function to use that gives this:

loadTexture :: String -> ErrorT String IO GL.GLuint
loadTexture filename = do
    p <- ErrorT $ loadPNGFile filename
    lift $ oglLoadImg p
    where
        oglLoadImg :: PNG.PNGImage -> IO GL.GLuint
        oglLoadImg png = do putStrLn "...I need todo IO stuff in here"
                            return 0

Now that we've merged the conceptual composite operation, we can begin condensing the specific operations more effectively. Collapsing the do block into monadic function application is a good start:

loadTexture :: String -> ErrorT String IO GL.GLuint
loadTexture filename = lift . oglLoadImg =<< ErrorT (loadPNGFile filename)
    where
        oglLoadImg :: PNG.PNGImage -> IO GL.GLuint
        oglLoadImg png = do putStrLn "...I need todo IO stuff in here"
                            return 0

And depending on what you're doing in oglLoadImg, you might be able to do more.

score 4 · Answer 3 · answered Aug 23 '11 at 20:51

Use an instance of Data.Traversable.Traversable for Either and then mapM. An instance could be:

instance Traversable (Either a) where
  sequenceA (Left x)  = pure $ Left x
  sequenceA (Right x) = Right <$> x

Now you can simply use forM:

loadTexture :: String -> IO (Either String GL.GLuint)
loadTexture filename = do
  p <- PNG.loadPNGFile filename
  forM p $ \p -> do
    -- Whatever needs to be done
  -- continue here.

dave4420 · Answer 4 · 2011-08-23T21:19:22.783

2

How about this?

loadTexture :: String -> IO (Either String GL.GLuint)
loadTexture filename = either (return . Left) oglLoadImg =<< PNG.loadPNGFile filename
    where
        oglLoadImg :: PNG.PNGImage -> IO (Either String GL.GLuint)
        oglLoadImg png = do -- IO stuff in here

(I'm not totally happy with the either (return . Left) bit and wonder whether it can be replaced with some sort of lift incantation.)

edited Aug 23 '11 at 21:19

answered Aug 23 '11 at 20:51

dave4420

46,404
6
118
152

Yes, `lift` is appropriate here. See the nearly-simultaneous answers by hammar and myself demonstrating `ErrorT` for details. :] – C. A. McCann Aug 23 '11 at 21:04
@tm1rbrt I ballsed up the type signature; hopefully fixed now. But hammer and C.A.McCann's answers are better. – dave4420 Aug 23 '11 at 21:21

Could i be using a bind/fmap here

4 Answers4