Simplification of way too long Haskell function-CodePudding

I wrote a function to query currency exchanges rate from an API. It works fine, but the code is way too long and unreadable. I thought someone would be able to help me simplify this, especially because there are many repeated patterns and operators like the repeated use of

... <&> (=<<) (something >>= pure) ...

I've just started learning Haskell and therefore don't know many clever operators/functions/lenses that could be used here.

Btw, I am aware that do-notation exists.

forex :: (String, String) -> IO (Maybe (Scientific, UnixTime))
forex cp = (get ("https://www.freeforexapi.com/api/live?pairs="    uncurry (  ) cp) <&> decode . flip (^.) responseBody <&> (=<<) (parseMaybe (.: "rates") >>= pure) :: IO (Maybe (Map Key (Map Key Scientific)))) <&> (=<<) (Data.Map.lookup (fromString (uncurry (  ) cp)) >>= pure) <&> (=<<) ((pure . toList) >>= pure) <&> (=<<) (pure . map snd >>= pure) <&> fmap (\y -> (head y, UnixTime ((CTime . fromRight 0 . floatingOrInteger) (y !! 1)) 0))

The received JSON looks like this

{"rates":{"EURUSD":{"rate":1.087583,"timestamp":1649600523}},"code":200}

Thanks in advance.

CodePudding user response：

Wow, that is too long. Let's take it step by step; by the end, we will arrive at the following code snippet which I find much more natural to read but which performs exactly the same computation:

forex (c, p) = extractFirstTime c p
    <$> get ("https://www.freeforexapi.com/api/live?pairs="    c    p)

extractFirstTime c p response = firstTime
    <$> parseAndLookUp c p (response ^. responseBody)

parseAndLookUp c p body =
    decode body >>=
    parseMaybe (.: "rates") >>=
    Data.Map.lookup (fromString (c    p))

firstTime = case Data.Map.elems m of
    k:t:_ -> (k, UnixTime ((CTime . fromRight 0 . floatingOrInteger) t) 0)

Let's see how.

To start, I think it's easier to see and edit if there are strategically chosen line breaks.

forex cp =
    (get ("https://www.freeforexapi.com/api/live?pairs="    uncurry (  ) cp)
        <&> decode . flip (^.) responseBody
        <&> (=<<) (parseMaybe (.: "rates") >>= pure)
        :: IO (Maybe (Map Key (Map Key Scientific)))
    )
    <&> (=<<) (Data.Map.lookup (fromString (uncurry (  ) cp)) >>= pure)
    <&> (=<<) ((pure . toList) >>= pure)
    <&> (=<<) (pure . map snd >>= pure)
    <&> fmap (\y -> (head y, UnixTime ((CTime . fromRight 0 . floatingOrInteger) (y !! 1)) 0))

One of the monad laws is m >>= pure = m, so let's delete >>= pure everywhere. (One each on lines 4, 7, 8, and 9.)

forex cp =
    (get ("https://www.freeforexapi.com/api/live?pairs="    uncurry (  ) cp)
        <&> decode . flip (^.) responseBody
        <&> (=<<) (parseMaybe (.: "rates"))
        :: IO (Maybe (Map Key (Map Key Scientific)))
    )
    <&> (=<<) Data.Map.lookup (fromString (uncurry (  ) cp))
    <&> (=<<) (pure . toList)
    <&> (=<<) (pure . map snd)
    <&> fmap (\y -> (head y, UnixTime ((CTime . fromRight 0 . floatingOrInteger) (y !! 1)) 0))

Another monad law is m >>= pure . f = fmap f m. Let's simplify with that law where possible. (One each on lines 8 and 9.)

forex cp =
    (get ("https://www.freeforexapi.com/api/live?pairs="    uncurry (  ) cp)
        <&> decode . flip (^.) responseBody
        <&> (=<<) (parseMaybe (.: "rates"))
        :: IO (Maybe (Map Key (Map Key Scientific)))
    )
    <&> (=<<) Data.Map.lookup (fromString (uncurry (  ) cp))
    <&> fmap toList
    <&> fmap (map snd)
    <&> fmap (\y -> (head y, UnixTime ((CTime . fromRight 0 . floatingOrInteger) (y !! 1)) 0))

The uses of uncurry are happening because we're not pattern-matching on cp. Let's fix that up. (Lines 1, 2, and 7.)

forex (c, p) =
    (get ("https://www.freeforexapi.com/api/live?pairs="    c    p)
        <&> decode . flip (^.) responseBody
        <&> (=<<) (parseMaybe (.: "rates"))
        :: IO (Maybe (Map Key (Map Key Scientific)))
    )
    <&> (=<<) Data.Map.lookup (fromString (c    p))
    <&> fmap toList
    <&> fmap (map snd)
    <&> fmap (\y -> (head y, UnixTime ((CTime . fromRight 0 . floatingOrInteger) (y !! 1)) 0))

My mental type-checker is going nuts. Let's split this calculation into three different kinds of things: one that works in IO, one that works in Maybe, and one that is pure. First let's split the IO from everything else.

forex (c, p) = extractFirstTime c p
    <$> get ("https://www.freeforexapi.com/api/live?pairs="    c    p)

extractFirstTime c p response = response
    & decode . flip (^.) responseBody
    & (=<<) (parseMaybe (.: "rates"))
    & (=<<) Data.Map.lookup (fromString (c    p))
    & fmap toList
    & fmap (map snd)
    & fmap (\y -> (head y, UnixTime ((CTime . fromRight 0 . floatingOrInteger) (y !! 1)) 0))

Now let's split out the Maybe parts.

forex (c, p) = extractFirstTime c p
    <$> get ("https://www.freeforexapi.com/api/live?pairs="    c    p)

extractFirstTime c p response = parseAndLookUp c p (response ^. responseBody)
    & fmap toList
    & fmap (map snd)
    & fmap (\y -> (head y, UnixTime ((CTime . fromRight 0 . floatingOrInteger) (y !! 1)) 0))

parseAndLookUp c p body =
    decode body >>=
    parseMaybe (.: "rates") >>=
    Data.Map.lookup (fromString (c    p))

And let's split out the pure parts. One of the functor laws is fmap f . fmap g = fmap (f . g), so we can merge the three fmaps in extractFirstTime. At that point, the two arguments to (&) that remain are short enough that we can inline the definition of (&). I'll also use the name (<$>) instead of fmap; I think it reads a bit clearer.

forex (c, p) = extractFirstTime c p
    <$> get ("https://www.freeforexapi.com/api/live?pairs="    c    p)

extractFirstTime c p response = firstTime
    <$> parseAndLookUp c p (response ^. responseBody)

parseAndLookUp c p body =
    decode body >>=
    parseMaybe (.: "rates") >>=
    Data.Map.lookup (fromString (c    p))

firstTime m = m
    & toList
    & map snd
    & (\y -> (head y, UnixTime ((CTime . fromRight 0 . floatingOrInteger) (y !! 1)) 0))

Data.Map has a name for map snd . toList, namely, elems. Instead of using head and !!, let's use pattern matching to pick out the elements we want. (All changes are in firstTime.)

forex (c, p) = extractFirstTime c p
    <$> get ("https://www.freeforexapi.com/api/live?pairs="    c    p)

extractFirstTime c p response = firstTime
    <$> parseAndLookUp c p (response ^. responseBody)

parseAndLookUp c p body =
    decode body >>=
    parseMaybe (.: "rates") >>=
    Data.Map.lookup (fromString (c    p))

firstTime = case Data.Map.elems m of
    k:t:_ -> (k, UnixTime ((CTime . fromRight 0 . floatingOrInteger) t) 0)

There are likely additional beautifying things that could be done (adding type signatures comes to mind, and I have several ideas that change/improve the behavior of the code), but I think by this point you have something that's fairly reasonable to read and understand. Along the way, making things readable has, as a side effect, eliminated the repeated code snippets you found unnerving, so that's a little bonus; but if they had remained, it would be very natural to try to address them as an additional step.