# How to refactor this conditional piping
**URL:** https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656
**Category:** Request Feedback
**Created:** [November 11, 2019, 7:04am UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656 "2019-11-11T07:04:34Z")
**Posts on this page:** 11
**Page:** 1
### Author:  [@kgashok](https://discourse.elm-lang.org/u/kgashok)
#### Post date: [November 11, 2019, 7:04am UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/1 "2019-11-11T07:04:34Z")
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?
```elm
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
)
```
---
### Author:  [@eike](https://discourse.elm-lang.org/u/eike)
#### Post date: [November 11, 2019, 8:49am UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/2 "2019-11-11T08:49:57Z")
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
```auto
mapAdjacent : (a -> a -> b) -> List a -> List b
mapAdjacent f list =
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.
---
### Author:  [@kgashok](https://discourse.elm-lang.org/u/kgashok)
#### Post date: [November 11, 2019, 9:19am UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/3 "2019-11-11T09:19:16Z")
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?
---
### Author:  [@eike](https://discourse.elm-lang.org/u/eike)
#### Post date: [November 11, 2019, 9:43am UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/4 "2019-11-11T09:43:57Z")
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.
---
### Author:  [@kgashok](https://discourse.elm-lang.org/u/kgashok)
#### Post date: [November 11, 2019, 10:25am UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/5 "2019-11-11T10:25:30Z")
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`):
```elm
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`?
---
### Author:  [@eike](https://discourse.elm-lang.org/u/eike)
#### Post date: [November 11, 2019, 10:34am UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/6 "2019-11-11T10:34:47Z")
You can do
```auto
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`.
---
### Author:  [@kgashok](https://discourse.elm-lang.org/u/kgashok)
#### Post date: [November 11, 2019, 11:00am UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/7 "2019-11-11T11:00:39Z")
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?
---
### Author:  [@dmy](https://discourse.elm-lang.org/u/dmy)
#### Post date: [November 11, 2019, 4:55pm UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/8 "2019-11-11T16:55:50Z")
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.
---
### Author:  [@kgashok](https://discourse.elm-lang.org/u/kgashok)
#### Post date: [November 11, 2019, 6:18pm UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/9 "2019-11-11T18:18:19Z")
Between
> [@eike](#):
>
> calculateDiff3 : List Int → List Int calculateDiff3 = calculateDiff \>\> List.Extra.groupsOf 3 \>\> List.map List.sum
and
```elm
calculateDiff3 lst =
lst |> calculateDiff |> List.Extra.groupsOf3 |> List.map List.sum
```
which is preferable? And why?
---
### Author:  [@eike](https://discourse.elm-lang.org/u/eike)
#### Post date: [November 11, 2019, 9:18pm UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/10 "2019-11-11T21:18:45Z")
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.
---
### Author:  [@system](https://discourse.elm-lang.org/u/system)
#### Post date: [November 21, 2019, 9:18pm UTC](https://discourse.elm-lang.org/t/how-to-refactor-this-conditional-piping/4656/11 "2019-11-21T21:18:46Z")
This topic was automatically closed 10 days after the last reply. New replies are no longer allowed.