[Haskell-cafe] Advice needed on how to improve some code

[Sylvain Henry]

I don't think you need to update record fields of a blank record: you
can create the record using applicative operators instead. Something
like:

parseDevicePLData :: Bool -> Get PayloadData
parseDevicePLData hasEv = do
   rawEvId <- if hasEv then getWord8 else return 0 -- I guessed the 0 value
   let evId           = toEnum (fromIntegral rawEvId .&. 0x7f)
   let statusFlag = testBit rawEvId 7
   mask <- getWord16be

  let parseMaybe e p = if testBit mask (fromEnum e) then Just <$> p
else return Nothing

  DevicePL hasEv statusFlag evId mask
    <$> parseMaybe D.GPS parseDeviceGPSData
    <*> parseMaybe D.GSM parseDeviceGSMData
    <*> parseMaybe D.COT parseDeviceCotData
    <*> ...

parseDevicePL :: Bool -> Get Payload
parseDevicePL hasEv = do
   ts <- parseTimestamp
   P.Payload "" (Just ts)  <$> parseDevicePLData hasEv

Then you can lift these "parsers" into you Parser monad only when you need it.

-- Sylvain

2015-04-16 8:37 GMT+02:00 Jeff <jeff at datalinktech.com.au>:
> Thanks Tom, David and Claude for your replies.
>
>
>
>> On 16 Apr 2015, at 4:03 pm, Tom Ellis <tom-lists-haskell-cafe-2013 at jaguarpaw.co.uk> wrote:
>>
>> The first thing you should do is define
>>
>>    parseDeviceGPSDataOf constructor parser setField =
>>       ( \pl' -> let pld = P.payloadData pl' in
>>                  if testBit mdm ( fromEnum constructor )
>>                    then
>>                      parser >>=
>>                      ( \s -> return ( pl' { P.payloadData = setField pld (Just s) } } ) )
>>                    else
>>                      return pl' )
>>
>> and your chain of binds will become
>>
>>    setgpsData pld = pld { P.gpsData = Just s }
>>    ...
>>
>>    parseDeviceDataOf D.GPS parseDeviceGPSData setgpsData >>=
>>    parseDeviceDataOf D.GSM parseDeviceDSMData setgsmData >>=
>>    parseDeviceDataOf D.COT parseDeviceCOTData setcotData >>=
>>    ...
>>
>> Then I would probably write
>>
>>    deviceSpecs = [ (D.GPS, parseDeviceGPSData, setgpsData)
>>                  , (D.GSM, parseDeviceDSMData, setgsmData)
>>                  , (D.COT, parseDeviceCOTData, setcotData) ]
>>
>> and turn the chain of binds into a fold.
>>
>
>
> I’ll do as you have suggested Tom.  Thanks.
>
> Jeff
>
> _______________________________________________
> Haskell-Cafe mailing list
> Haskell-Cafe at haskell.org
> http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe