UPenn CIS 194 Homework 2: Log file parsing

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 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) = filterEmp

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?

StackExchange Code Review Q#104895, answer score: 2