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

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 :

Thanks Tom, David and Claude for your replies.

...

On 16 Apr 2015, at 4:03 pm, Tom Ellis 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@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe