Simple command line todo-list

Question

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?

• Syd Kerckhove: Hey would you like some help? This seems like an interesting project!
• lightandlight: @SydKerckhove Sure, hit me up on Google+ and we'll discuss what to do.

Answer 1

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)

• lightandlight: 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...
• itsbruce: It's a chain of mapped functions. Of course it works ;-)