I am working through the online materials for CIS 194, a upenn haskell class (not taking the class). This exercise is to parse an error log, and it gives a few function signatures to implement, ending with the very last one whatWentWrong. Please help me make the code more idiomatic, particularly whatWentWrong. My haskell level is: understood LYAH but have not written much code.

{-# OPTIONS_GHC -Wall #-}
module LogAnalysis where

import Log -- https://github.com/dustingetz/sandbox/tree/master/cis194/hw2

parseMessage :: String -> LogMessage
parseMessage s = case words s of
  -- I 5053 pci_id: con ing!
  ("I":t:msg) -> LogMessage Info (read t) (unwords msg)

  -- W 3654 e8] PGTT ASF!
  ("W":t:msg) -> LogMessage Warning (read t) (unwords msg)

  -- E 47 1034 'What a pity it
  ("E":sev:t:msg) -> LogMessage (Error $ read sev) (read t) (unwords msg)

  _ -> Unknown s

parse :: String -> [LogMessage]
parse s = map parseMessage $ lines s

insert :: LogMessage -> MessageTree -> MessageTree
insert (Unknown _) node = node
insert m Leaf = Node Leaf m Leaf
insert m@(LogMessage _ t _) (Node l' m'@(LogMessage _ t' _) r')
  | t <= t' = Node (insert m l') m' r'
  | t > t' = Node l' m' (insert m r')

build :: [LogMessage] -> MessageTree
build ms = foldl (flip insert) Leaf ms

inOrder :: MessageTree -> [LogMessage]
inOrder (Node l m r) = inOrder l ++ [m] ++ inOrder r
inOrder Leaf = []

--fmap ((take 10) . inOrder . build) $ testParse parse 100 "error.log"

whatWentWrong :: [LogMessage] -> [String]
whatWentWrong ms = map (\(LogMessage _ _ s) -> s) $
                   filter (\(LogMessage msgType _ _) ->
                            case msgType of
                             Error sev -> sev >= 50
                             _ -> False) $
                   inOrder $ build ms

1 Answer 1


Concerning whatWentWrong, the main problem is that the function isn't total. This means that for Unknown it will fail badly and unconditionally.

If you declare the functions from map and filter as named functions in a where (or let block), you will be able to check for Unknown as well in pattern matching. Also, you can combine several constructors in a pattern.

There are two options how to fix map: Either return some String for Unknown, or remove such entries. I'll show an example for the latter option using mapMaybe:

whatWentWrong :: [LogMessage] -> [String]
whatWentWrong = mapMaybe getMsg
                . filter filterExp
                . inOrder . build
    filterExp (LogMessage (Error sev) _ _) = sev >= 50
    filterExp _                            = False
    getMsg (LogMessage _ _ s) = Just s
    getMsg _                  = Nothing

Also note that you can declare this function as a composition of functions, in so-called point-free style.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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