4
\$\begingroup\$

I am working through UPenn CIS 194: Introduction to Haskell (Spring 2013). Since I am not able to take the course for real I am asking for CR (feedback) as it could be from teacher in that course.

HW2 - Log file parsing - Full description

{-# OPTIONS_GHC -Wall #-} module LogAnalysis where import Log parseMessage :: String -> LogMessage parseMessage x = case words x of "E":s:t:m -> LogMessage (Error (read s::Int)) (read t::Int) (unwords m) "W":t:m -> LogMessage Warning (read t::Int) (unwords m) "I":t:m -> LogMessage Info (read t::Int) (unwords m) r -> Unknown (unwords r) parse :: String -> [LogMessage] parse x = case lines x of f:xs -> [parseMessage f] ++ parse (unlines xs) [] -> [] insert :: LogMessage -> MessageTree -> MessageTree insert (Unknown _) t = t insert m Leaf = (Node Leaf m Leaf) insert message@(LogMessage _ t1 _) (Node l root@(LogMessage _ t2 _) Leaf) | t1 > t2 = (Node l root (Node Leaf message Leaf)) insert message@(LogMessage _ t1 _) (Node Leaf root@(LogMessage _ t2 _) r) | t1 <= t2 = (Node (Node Leaf message Leaf) root r) insert message@(LogMessage _ t1 _) (Node l root@(LogMessage _ t2 _) r) | t1 > t2 = (Node l root (insert message r)) | otherwise = (Node (insert message l) root r) build :: [LogMessage] -> MessageTree build [] = Leaf build (x:xs) = insert x (build xs) inOrder :: MessageTree -> [LogMessage] inOrder Leaf = [] inOrder (Node left message right) = (inOrder left) ++ [message] ++ (inOrder right) whatWentWrong :: [LogMessage] -> [String] whatWentWrong log = (filterEmpty (errorsWithSeverity50 (inOrder (build log)))) errorWithSeverity :: LogMessage -> Int -> String errorWithSeverity (LogMessage (Error s) _ m) severity | s >= severity = m | otherwise = "" errorWithSeverity _ _ = "" errorsWithSeverity50 :: [LogMessage] -> [String] errorsWithSeverity50 [] = [] errorsWithSeverity50 (x:xs) = [(errorWithSeverity x 50)] ++ errorsWithSeverity50 xs filterEmpty :: [String] -> [String] filterEmpty [] = [] filterEmpty ("":xs) = filterEmpty xs filterEmpty (x:xs) = [x] ++ filterEmpty xs
\$\endgroup\$
0

1 Answer 1

Reset to default
2
\$\begingroup\$

parseMessage

The main problem here is that you are calling unwords on the result of words ..., and this doesn't preserve the original whitespace.

For instance, the last case should just be:

case words x of ... _ -> Unknown x -- use the original message text

and you have a similar problem with unwords m.

Unfortunately, to preserve whitespace in these cases I don't think you will be able to use the words function here.

It is customary to have parens around the list patterns, e.g.:

parseMessage x = case words x of ("E":s:t:m) -> LogMessage (Error (read s::Int)) (read t::Int) (unwords m)

Even though they are not needed, they aid in reading the code.

Also, aligning the -> in the case patterns helps with readability.

The type signatures on your read calls are not needed due to Haskell's type inference.

insert

You should be getting this warning from ghc and ghci:

LogAnalysis.hs:19:1: Warning: Pattern match(es) are non-exhaustive In an equation for ‘insert’: Patterns not matched: (LogMessage _ _ _) (Node (Node _ _ _) (Unknown _) _) (LogMessage _ _ _) (Node Leaf (Unknown _) _)

This means that your code doesn't cover all of the possible cases.

Append to your question an updated version of insert which doesn't produce any warnings, and then I might have some further comments about it.

(Please append to your question instead of modifying it so it doesn't invalidate any existing comments.)

Let me know if you are not seeing this warning from ghc / ghci so we can figure out what's going on with your environment.

filterEmpty

I don't think the logic is right here. What if you have this message:

LogMessage (Error 62) 23 ""

Shouldn't the empty message be included in the message list?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thanks, really appreciate your help! Updated version of code is available at public gist file \$\endgroup\$ Commented Sep 17, 2015 at 21:09

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.