 # How to refactor this conditional piping

1. This code doesn’t seem idiomatic, where the piping is based on the `inter` boolean value. How can it be improved?
2. Not that it is important, but is it possible to refactor and avoid the `List.Extra` function and still get the same effect?
``````calculateDiff : List ( Int, Int ) -> Bool -> List ( Int, Int )
calculateDiff tlst inter =
let
( _, lst ) =
List.unzip tlst

ziplist =
List.map2 Tuple.pair lst (withDefault [] (List.tail lst))
|> List.map (\( x, y ) -> y - x)

lst3 =
ziplist |> List.Extra.groupsOf 3 |> List.map List.sum
in
List.indexedMap Tuple.pair
(if inter then
ziplist

else
lst3
)
``````

I think your problems come from the fact that this function does a lot of things on different levels:

1. It throws away the first component of each entry. (This seems like data preparation and should maybe be part of some parsing step earlier?)
2. It contains an ad-hoc implementation of function to map over adjacent pairs in a list. (This should be its own function.)
3. It calculates the pairwise differences. (This is what you want, I think.)
4. It contains logic to decide whether to calculate sums of groups of three. (This seems like an attempt to roll two different functions into one. Maybe it is also a choice based on some user-interface switch? I don’t know the surrounding code.)
5. It adds first components again. (This seems superfluous.)

I would probably go for something like

``````mapAdjacent : (a -> a -> b) -> List a -> List b
List.map2 f list (withDefault [] (List.tail list))

calculateDiff : List Int -> List Int
calculateDiff = mapAdjacent (\x y -> y - x)

calculateDiff3 : List Int -> List Int
calculateDiff3 = calculateDiff >> List.Extra.groupsOf 3 >> List.map List.sum
``````

and then try to remove the first components earlier, i.e. at the call site. By the way, I think `List.map Tuple.second tlst` is the more elegant way to do that. I would also not add the indices to the list because they are implicitly contained in the order of the list. If you need them later, you can use `List.indexedMap` where you actually need them. Lastly, I would also move the `if` that decides whether to use `calculateDiff` or `calculateDiff3` to the call site.

4 Likes

You have amazingly read my mind! And I think I understand all your suggestions!
Thanks much!

As for the use of `List.Extra.groupsOf` function, is that reasonable?

You’re welcome. If what you want is to add groups of three (i.e. turn `[1,2,3,4,5,6,7]` into `[1+2+3, 4+5+6]`) then `List.Extra.groupsOf` is the way to go, I think. Note that this function just discards elements at the end if there are not enough for a final group. If you want something else, there is also `List.Extra.greedyGroupsOf` or you might have to roll your own.

1 Like

The refactor worked perfectly!

`List.Extra.groupsOf` is perfect for my use case, so I will stick with that.

In my calling site, I now have this (`lst` being the output of `calculateDiff` or `calculateDiff3`):

``````   lst
|> List.indexedMap Tuple.pair
|> List.map (\t -> ( t, String.fromInt (value t) ++ ", " ))
|> --- ...further piping
``````
1. Can the `indexedMap` and `map` be refactored to a single piping sequence?|
2. What is the meaning of the annotation `(a -> a -> b)` as found in `mapAdjacent`?

You can do

``````lst
|> List.indexedMap (\index val -> ( (index, val), String.fromInt (value (index, val)) ++ ", " ))
``````

where you can of course change `value (index, val)` to simply `val` if the function `value` just extracts the second component again. By the way, if `value` does not use the index, I would argue that your use of `indexedMap` is still to early. I would put it right before the point where I actually need the index.

The `(a -> a -> b)` in the type of `mapAdjacent` means that the first parameter is a function that takes two values of the same type `a` and returns a value of a (potentially, but not necessarily) different type `b`. In your case, both `a` and `b` are simply `Int` because the function `(\x y -> y - x)` turns two `Int`s into one `Int`.

1 Like

Is it `(\index val ->`

or do I need a comma in between `index` and `val`?
Sorry, my bad. It did compile without the `,`.

When does one use a comma and when is it not required?

If I’m not forgetting something, commas are only used in Elm:

• between list items, `[1, 2, 3]`
• between tuple items: `(1, 2, 3)`
• between record fields: `{firstname = "foo", lastname = "bar"}`
• when `exposing` symbols: `import Html exposing (Html, div, text)`

That’s all. Never use a comma else, including between functions or lambdas arguments.

1 Like

Between

and

``````calculateDiff3 lst =
lst |> calculateDiff |> List.Extra.groupsOf3 |> List.map List.sum
``````

which is preferable? And why?

Both of them do the same thing. I like that the first one is more concise and that I don’t have to think of a name for the variable. But you might find the second one more readable and then you should probably go with that one.

This topic was automatically closed 10 days after the last reply. New replies are no longer allowed.