Log messages parser

I've been studying Haskell for some time, but I have never used it for real. So, I started following the CIS 194 Course from UPenn. I resolved the second homework but I'm not fully satisfied with the parser I came up with. I think it's way too verbose and I don't like the way the message string is passed around. I am also not sure if the code is being idiomatic.

Is it following Haskell standards and being idiomatic?

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

import Log
import Text.Read

getErrorSeverity :: Maybe Int -> Maybe MessageType
getErrorSeverity (Just c) = Just (Error c)
getErrorSeverity _        = Nothing

parseMessageType :: String -> Maybe MessageType
parseMessageType "" = Nothing
parseMessageType m = case words m of
    "I":_   -> Just Info
    "W":_   -> Just Warning
    "E":l:_ -> getErrorSeverity (readMaybe l)
    _       -> Nothing

parseTimeStamp :: String -> Maybe TimeStamp
parseTimeStamp m = case words m of
    "E":_:s:_ -> readMaybe s
    "I":s:_   -> readMaybe s
    "W":s:_   -> readMaybe s
    _         -> Nothing

parseLogContent :: String -> Maybe String
parseLogContent m = case words m of
    "E":_:_:r -> Just (unwords r)
    _:_:r     -> Just (unwords r)
    _         -> Nothing

parseMessage :: String -> LogMessage
parseMessage "" = Unknown ""
parseMessage m = case (parseMessageType m, parseTimeStamp m, parseLogContent m) of
    (Just t, Just ts, Just c) -> LogMessage t ts c
    _      -> Unknown m

Disclaimer: I assume you want to write confident idiomatic Haskell regardless if an assignment guarantees a perfect log file format. Because, in the real world of software development there are no such guarantees. Better to build the habit now while you're learning rather than later when you're pressed to meet a deadline.

In light of my disclaimer, I must disagree with the answer you accepted. It is bad advice to say that one should avoid Maybe. In fact, I would argue that the code you presented is better than the code you accepted as the answer.

Why?

The parseMessage you wrote is a total function as opposed to a partial function. Let's see what others have to say about this:

• Partial functions

• Avoiding partial functions

• Are your functions total?

Now that we agree on the use of total functions let's try to refactor the code while keeping parseMessage a total function.

There are two main ways parseMessage can fail to parse a message:

• The log line isn't in the info, warning or error format.

• The timestamp or severity isn't given as an integer.

The 1st problem we can handle via pattern matching:

parseMessage :: String -> LogMessage
parseMessage m =
  case (words m) of
    "I":t:ws   -> undefined
    "W":t:ws   -> undefined
    "E":s:t:ws -> undefined
    _          -> Unknown m

The 2nd problem we can isolate as follows:

parseInt :: String -> Maybe Int
parseInt s =
  case (reads s :: [(Int, String)]) of
    [(n, "")] -> Just n
    _         -> Nothing

Note the use of Maybe to explicitly declare that this function may fail to parse an Int. It is a total function.

To learn more about reads check out this wonderful article.

Let's now make a bold assumption so that we don't have to deal with Maybe explicitly in our code.

Let's assume we always have an Int when it is needed. For e.g. when we need the timestamp as an Int let's assume we have it.

Doing so allows us to write the following functions:

info c t    = LogMessage Info t c
warning c t = LogMessage Warning t c
err c s t   = LogMessage (Error s) t c

Now, this doesn't quite work:

info (unwords ws) (parseInt t)

Because parseInt returns Maybe Int and info only accepts an Int as its 2nd argument. What we really need is:

asInfo :: String -> Maybe Int -> Maybe LogMessage
asInfo _ Nothing -> Nothing
asInfo c (Just t) -> Just (info c t)

Well, that is what liftA and liftA2 can do for us:

asInfo c    = liftA $ info c
asWarning c = liftA $ warning c
asErr c     = liftA2 $ err c

Finally, when we use asInfo (unwords ws) (parseInt t) we don't want to get back Nothing in the case of a failure. We really want to return Unknown m. We can use maybe for that:

maybe (Unknown m) id (asInfo (unwords ws) (parseInt t))

Putting everything together we get:

import Control.Applicative (liftA, liftA2)

parseMessage :: String -> LogMessage
parseMessage m =
  case (words m) of
    "I":t:ws   -> try $ asI (unwords ws) $ parseInt t
    "W":t:ws   -> try $ asW (unwords ws) $ parseInt t
    "E":s:t:ws -> try $ asE (unwords ws) (parseInt s) $ parseInt t
    _          -> u
  where
    try = maybe u id

    asI c = liftA $ i c
    asW c = liftA $ w c
    asE c = liftA2 $ e c

    i c t   = LogMessage Info t c
    w c t   = LogMessage Warning t c
    e c s t = LogMessage (Error s) t c
    u       = Unknown m

parseInt :: String -> Maybe Int
parseInt s =
  case (reads s :: [(Int, String)]) of
    [(n, "")] -> Just n
    _         -> Nothing

Note how the main part of the code captures the essence of the problem we're trying to solve while we still get to explicitly handle any errors that may occur and remain confident about our code.

case (words m) of
    "I":t:ws   -> try $ asI (unwords ws) $ parseInt t
    "W":t:ws   -> try $ asW (unwords ws) $ parseInt t
    "E":s:t:ws -> try $ asE (unwords ws) (parseInt s) $ parseInt t
    _          -> u

That I believe is the beauty of Haskell.

StackExchange Code Review Q#117351, answer score: 9