Haskell todo list

Question

I don't have much experience with Haskell or functional programming, but I wrote a working todo list. My main question is if there's any idiomatic shortcuts I could take and if my code is idiomatic enough, but all constructive criticism is welcome. Tested in GHCi 7.6.3:

import Data.Char
import System.IO

numberList :: [String] -> [String]
numberList = zipWith (\a b -> a ++ ". " ++ b) (map show [1..])

removeElem :: Int -> [a] -> [a]
removeElem i xs = take i xs ++ drop (succ i) xs

printStrs :: [String] -> IO ()
printStrs = mapM_ putStrLn

todoOp :: [String] -> Char -> IO [String]
todoOp xs 'V' = do
  if (not.null) xs then printStrs $ numberList xs
  else putStrLn "No entries"
  return xs
todoOp xs 'A' = do
  putStrLn "What would you like to add?"
  str <- getLine
  return (str:xs)
todoOp xs 'D' = do
  if null xs then do
    putStrLn "No entries"
    return xs
  else do
    printStrs $ numberList xs
    putStr "Which line do you want to remove? "
    i <- readLn :: IO Int
    return (removeElem (pred i) xs)
todoOp xs _ = do
    putStrLn "Operation not supported"
    return xs

mainLoop xs = do
  putStr "Do you wish to [a]dd, [d]elete, [v]iew, or [e]xit? "
  c <- getChar
  putChar '\n'
  if (toUpper c) == 'E' then return ()
  else do
    xs' <- todoOp xs (toUpper c)
    mainLoop xs'

main :: IO ()
main = do
    mainLoop []

Answer 1

There are some points that can be improved:

• a type synonym like type TodoList = [String] makes it easier to change the type later
• the IO and non-IO part in todoOp could be split
• System.IO is not used (getChar and putChar are in Prelude)
• mainLoop is missing a type signature
• removeElem can be written with splitAt
• show can be applied in zipWith's first argument, no need for an additional map there

If we apply those improvements, we end up with

import Data.Char (toUpper)

type Todo     = String
type TodoList = [Todo]

numberList :: TodoList -> [String]
numberList = zipWith (\a b -> show a ++ ". " ++ b) [1..]

removeElem :: Int -> [a] -> [a]
removeElem i xs = let (as, bs) = splitAt i xs
                  in as ++ drop 1 bs

printStrs :: [String] -> IO ()
printStrs = mapM_ putStrLn

--------------------------------------------
-- new functions to have todoOp just deal --
-- with IO and have testable functions    --

showTodoList :: TodoList -> String
showTodoList [] = "No entries"
showTodoList xs = unlines . numberList

addTodo :: TodoList -> Todo -> TodoList
addTodo = flip (:)

removeTodo :: TodoList -> Int -> TodoList
removeTodo = flip removeElem

--------------------------------------------

-- in case your GHC is too old, import Data.Functor for <$>

todoOp :: TodoList -> Char -> IO TodoList
todoOp xs 'V' = putStrLn (showTodoList xs) >> return xs
todoOp xs 'A' = do
  putStrLn "What would you like to add?"
  addTodo xs <$> getLine
todoOp [] 'D' = putStrLn (showTodoList []) >> return []
todoOp xs 'D' = do
    printStrs $ numberList xs
    putStr "Which line do you want to remove? "
    removeTodo xs <$> readLn
todoOp xs _ = do
    putStrLn "Operation not supported"
    return xs

If we're going for brevitiy, a prompt function might come handy, but that's a matter of preference:

prompt :: String -> IO String
prompt xs = putStrLn xs >> getLine

todoOp :: TodoList -> Char -> IO TodoList
todoOp xs 'V' = printTodoList xs >> return xs
todoOp xs 'A' = addTodo xs <$> prompt "What would you like to add?"
todoOp [] 'D' = putStrLn (showTodoList []) >> return []
todoOp xs 'D' = printTodoList xs >> removeTodo xs <$> prompt "Which line do you want to remove? "
todoOp xs _   = putStrLn "Operation not supported" >> return xs

However, there is still one thing amiss. todoOp has to handle every Char, although that shouldn't be necessary. There are only four actions: add, show, remove and quit. The latest does not actually act on a todo list, so let us focus on the first three. We should model them into a type:

data Action = Add | Delete | View

charToAction :: Char -> Maybe Action
charToAction x = case toUpper x of
                   'A' -> Just Add
                   'V' -> Just View
                   'D' -> Just Delete
                   _   -> Nothing

We have to adjust todoOp of course, but that's left as an exercise. mainLoop will now handle the parsing:

mainLoop :: TodoList -> IO ()
mainLoop xs = do
  putStr "Do you wish to [a]dd, [d]elete, [v]iew, or [e]xit? "
  c <- getChar
  putStrLn ""

  case charToAction c of
    Nothing | toUpper c == 'E' -> return ()
    Nothing   -> putStrLn "Unsupported operation" >> mainLoop xs
    Just act  -> todoOp xs act >>= mainLoop

Alternatively, you can use when from Control.Monad:

mainLoop :: TodoList -> IO ()
mainLoop xs = do
  putStr "Do you wish to [a]dd, [d]elete, [v]iew, or [e]xit? "
  c <- getChar
  putStrLn ""

  when (toUpper c /= 'E') $ case charToAction c of
    Nothing   -> putStrLn "Unsupported operation" >> mainLoop xs
    Just act  -> todoOp xs act >>= mainLoop