0

I'm trying to simplify the below code that's part of an attoparsec parser for a network packet, and I'm at a loss for a nice way to do it.

It starts with a call to atEnd :: IO Bool to determine if there's more to parse. I can't find a nicer way to use atEnd than to unwrap it from IO and then use it in an if statement, but it seems like there must be be a simpler way to check bool inside a monad. Here's the code:

maybePayload :: Parser (Maybe A.Value)
maybePayload = do
    e <- atEnd
    if e then return Nothing
    else do
        payload' <- char ':' *> takeByteString
        maybe mzero (return . Just) (maybeResult $ parse A.json payload')

The intention is to return Nothing if there is no payload, to return Just A.Value if there is a valid JSON payload, and for the parser to fail if there is a non-valid payload.

Here's the Packet record that eventually gets created:

data Packet = Packet
    { pID       :: Integer
    , pEndpoint :: String
    , pPayload  :: Maybe A.Value
    }
jhickner
  • 1,043
  • 10
  • 15

2 Answers2

3

You're doing a lot of work you don't need to do. First you check if you're at the end of the data and return Nothing if that doesn't work out. That's just not necessary, because if you're at the end, any parser that requires content will fail, and using maybeResult will turn that failure into Nothing.

The only time your parser fails is with the case where the input has data which doesn't start with the character :, the rest of the time it succeeds, even if that's by returning Nothing.

The only actual parsing going on is checking for : then using A.json. I think you're trying to write the whole program inside one parser, whereas you should just do the parsing on its own then call that as necessary. There's no need to check for end of data, or to make sure you get the whole content - that's all built in for free in a parser. Once you get rid of all that unnecessary checking, you get

payload :: Parser A.Value
payload = char ':' *> A.json

If you want to you can use that as maybeResult $ parse payload input to get a Maybe A.Value that's not additionally wrapped in a Parser. If you don't apply maybeResult, you can pattern match on the Result returned to deal separately with Failure, Partial success and Success.

Edit: OK, clearer now, thanks:

  • (If there's a colon followed by invalid json, fail)
  • If there's a colon followed by valid json, succeed, wrapping it in Just
  • If there's just end of input, succeed, returning Nothing

So we get:

maybePayload :: Parser (Maybe A.Value)
maybePayload = char ':' *> (Just <$> A.json)
            <|> (Nothing <$ endOfInput)

I've used <$> and <$ from Control.Applicative, or if you prefer, from Data.Functor.

<$> is an infix version of fmap, so Just <$> A.json does A.json and wraps any output in Just.

<$ is fmap.const so replaces the () from endOfInput with Nothing.

AndrewC
  • 32,300
  • 7
  • 79
  • 115
  • I edited the question to include the record I'm parsing into, which will hopefully make the intention more clear. If there's no payload but the rest of the packet parses, that's fine, but if there's something there that's not a valid payload, the packet parse will fail. – jhickner Dec 05 '12 at 00:39
  • @jhickner I think I get you - is the edit what you were after? – AndrewC Dec 05 '12 at 02:10
0

Why you need to encode failure in Maybe when the parser monad already has a built-in notion of failure? The problem with using Maybe in this way is that the parser cannot backtrack.

You could try something like this (I haven't tried to typecheck it) and then use option in the caller:

payload :: Parser Value
payload = do
  payload' <- char ':' *> takeByteString
  let res = parse A.json payload'
  case res of
    Error msg -> fail msg
    Success a -> return a
jaket
  • 9,140
  • 2
  • 25
  • 44
  • I'm intentionally discriminating between the case when there's no payload, in which case I want ```Nothing``` returned (it ends up as the value of a payload field in a record), and the case where there is some payload present but it's not valid JSON, in which case I want the parsing of the entire packet to fail. – jhickner Dec 05 '12 at 00:30