1
\$\begingroup\$

Introduction

I've written a simple command line todo-list in Haskell.

The full code can be found here. However, given people's time constraints, I have selected three verbose functions for review.

Function One

program :: StateT TodoList IO ()
program = do
    StateT $ \xs -> do
        (choice,_) <- runStateT requestChoice xs
        (_,xs') <- runStateT (parseChoice choice) xs
        runStateT printList xs'
        runStateT program xs'

main = runStateT program []

Questions:

  • Am I repeating myself too much by extracting the monads using runStateT?
  • Is there a better way to structure the main program?

Function Two

parseChoice :: Text -> StateT TodoList IO ()
parseChoice choice
    | first == "add" = addItem $ TodoItem second third
    | first == "remove" = removeItem $ parseInt second
    | first == "save" = saveList second
    | first == "load" = loadList second
    | first == "quit" = StateT $ \_ -> exitSuccess
    | otherwise = invalidChoice first
    where   args =  split (=='|') (toLower choice)
        first = head args
        second = if length args > 1 then args !! 1 else ""
        third = if length args > 2 then args !! 2 else ""

Questions:

  • Am I using too many guards?

Function Three

removeItem :: Maybe Int -> StateT TodoList IO ()
removeItem mn = StateT remove
    where remove = \xs -> case mn of
        Just n -> if n <= length xs - 1 
            then return ((),removeAt n xs)
            else do
                S.putStrLn "Index too large"
                return ((),xs)
        Nothing -> do
            S.putStrLn "Invalid index entered"
            return ((),xs)

General Questions

  • Which parts are un-idiomatic?
  • Which parts are superfluous?
  • Should I decompose these longer functions into smaller functions that do less?
  • How else can I improve this code?
\$\endgroup\$
  • \$\begingroup\$ Hey would you like some help? This seems like an interesting project! \$\endgroup\$ – Syd Kerckhove Jan 26 '15 at 14:46
  • \$\begingroup\$ @SydKerckhove Sure, hit me up on Google+ and we'll discuss what to do. \$\endgroup\$ – lightandlight Jan 27 '15 at 6:54
1
\$\begingroup\$

It seems like you may not quite grasp the interplay between do-notation and monad transformer stacks. Take a look at how I've rewritten program here to leverage the actual machinery of StateT. The version you wrote is needlessly verbose due to your manually plumbing the state around!

program :: StateT TodoList IO ()
program = do
    choice <- requestChoice
    parseChoice choice
    printList
    program

parseChoice can be cleaned up by favoring pattern matching over guards. Whenever you see a wall of guards that depend only on Eq, consider pattern matching instead.

parseChoice :: Text -> StateT TodoList IO ()
parseChoice choice =
    case split (== '|') (toLower choice) of
        ["add", date, message] -> addItem $ TodoItem date message
        ["remove", index]      -> removeItem $ parseInt index
        ["save", file]         -> saveList file
        ["load", file]         -> loadList file
        ["quit"]               -> lift exitSuccess
        (invalid:_)            -> invalidChoice invalid

I think you can probably guess what can change about removeItem after reading my other changes now, so before reading this next code block try rewriting it on your own.

removeItem :: Maybe Int -> StateT TodoList IO ()
removeItem Nothing  = lift $ putStrLn "Invalid index entered"
removeItem (Just n) = modify (removeAt n)
\$\endgroup\$
  • \$\begingroup\$ Thank you. I didn't realise that m >>= f passed the result of running the StateT to f (I thought it passed the state). After perusing the source code I properly understand how to properly chain the transformer. And might I add - Wow! How elegantly designed is this!? Bind practically ignores the inner monad! It works so well... \$\endgroup\$ – lightandlight Jan 27 '15 at 5:17
  • \$\begingroup\$ It's a chain of mapped functions. Of course it works ;-) \$\endgroup\$ – itsbruce Jan 27 '15 at 8:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.