Implementing Todo in Haskell

Question

Per Learn You a Haskell, I implemented a simple command-line program, Todo, that accepts 1 of 3 arguments: view or add or delete.

Please review my code for Haskell idioms & overall quality.

-- view, add and delete tasks
-- borrows delete from previous LYAH exercise
import System.Environment
import System.Directory
import System.IO
import Data.List

data Action = Add | View | Delete

main = do
    args <- getArgs
    progName <- getProgName
    let maybeAction = validArgs args
    executeTodo maybeAction

executeTodo :: (Maybe Action) -> IO ()
executeTodo (Just Add)    = addTodo
executeTodo (Just View)   = viewTodo
executeTodo (Just Delete) = deleteTodo
executeTodo Nothing       = print "invalid user input"

viewTodo :: IO ()
viewTodo = do
    withFile "todo_file.txt" ReadMode (\handle -> do 
        contents <- hGetContents handle
        putStr contents)
    return ()

addTodo :: IO ()
addTodo = do
   todoItem <- getLine
   appendFile "todo_file.txt" (todoItem ++ "\n")    

deleteTodo :: IO () 
deleteTodo = do        
    handle <- openFile "todo_file.txt" ReadMode  
    (tempName, tempHandle) <- openTempFile "." "temp"  
    contents <- hGetContents handle  
    let todoTasks = lines contents     
        numberedTasks = zipWith (\n line -> show n ++ " - " ++ line) [0..] todoTasks     
    putStrLn "These are your TO-DO items:"  
    putStr $ unlines numberedTasks  
    putStrLn "Which one do you want to delete?"     
    numberString <- getLine     
    let number = read numberString     
        newTodoItems = delete (todoTasks !! number) todoTasks     
    hPutStr tempHandle $ unlines newTodoItems  
    hClose handle  
    hClose tempHandle  
    removeFile "todo_file.txt"  
    renameFile tempName "todo_file.txt"  

validArgs :: [String] -> Maybe Action
validArgs (x:[]) = isValidAction x
validArgs _      = Nothing

isValidAction :: String -> Maybe Action
isValidAction x
  | x == "view"   = Just View
  | x == "add"    = Just Add
  | x == "delete" = Just Delete
  | otherwise     = Nothing

Answer 1

It looks good overall.

I see two possible problems.

Race condition in deleteTodo

Running the program twice at the same time you might lose one change because reading the old file, writing the new file and renaming is not atomic.

You can avoid that problem when using a "tododir" with one single file per item. It will also scale better with the number of todo items. Note, that that's very similar to why maildir is superior to mbox. See the rationale at djb's site.

Error handling is missing

You don't catch exceptions when doing file operations and use the partial read. For that simple interactive program I would not catch file operation errors but would surely use readMaybe.

Answer 2

do 
    contents <- hGetContents handle
    putStr contents

can become

hGetContents handle >>= putStr

and

validArgs (x:[]) = isValidAction x

can become

validArgs [x] = isValidAction x

and if you want, you can use pattern matching on strings rather than guards in that last function, but that's just my personal preference. I use guards to avoid a ton of nested ifs, and pattern match in the function definition if possible.