Deprecate result.partition in favor of new list.partition_map #3265
Replies: 7 comments
-
What's the motivation for deprecating this when we already have map and partition? |
Beta Was this translation helpful? Give feedback.
-
The reason I suggest deprecating Tangent about the performance of different approachesA valid concern that was pointed out to me in the Discord is that Looking into this, I ran a few naive tests using But that's probably a less relevant tangent anyway. If you actually care about those results I'd be happy to clean up and publish the code I'm testing with. Personally the reason I care about [#("wibble", 0), #("wobble", 1), #("wabble", 2)]
|> partition_map(fn(a) {
case a.1 |> int.is_odd {
True -> Ok(a.0)
False -> Error(a.1)
}
}) Is simpler than: [#("wibble", 0), #("wobble", 1), #("wabble", 2)]
|> list.map(fn(a) {
case a.1 |> int.is_odd {
True -> Ok(a.0)
False -> Error(a.1)
}
})
|> list.reverse
|> result.partition And significantly simpler than: [#("wibble", 0), #("wobble", 1), #("wabble", 2)]
|> list.partition(fn(a) { a.1 |> int.is_odd })
|> pair.map_first(fn(evens) { evens |> list.map(pair.first) })
|> pair.map_second(fn(odds) { odds |> list.map(pair.second) }) The 2nd approach being the slowest and counterintuitively requiring a The 3rd approach also becomes significantly more cumbersome if you're partitioning a list of [Ok(0), Error(1), Ok(2)]
|> list.partition(result.is_ok)
|> pair.map_first(fn(oks) {
oks
|> list.map(fn(a) {
let assert Ok(inner) = a
inner
})
})
|> pair.map_second(fn(errors) {
errors
|> list.map(fn(e) {
let assert Error(inner) = e
inner
})
}) Of course, for this you'd probably just use [Error(0), Ok("wobble"), Error(2)] |> partition_map(fn(a) { a }) TL;DRIt's my understanding that Gleam strives to provide a minimal number of tools, and that each of those tools be highly adaptable and widely applicable. I believe I apologize for the text wall, I'm just trying to be as clear and thorough as possible on my reasoning. |
Beta Was this translation helpful? Give feedback.
-
If you don't believe there's sufficient reason to deprecate |
Beta Was this translation helpful? Give feedback.
-
I don't personally find the "simpler" examples given here more compelling than the other examples, but perhaps it's still useful. Perhaps it should be a tuple of a bool and a value rather than a result as it's unrelated to failure. |
Beta Was this translation helpful? Give feedback.
-
I was following the precedent set by |
Beta Was this translation helpful? Give feedback.
-
On second thought, a pair would limit the output lists to being the same type. You'd need a |
Beta Was this translation helpful? Give feedback.
-
Good point on the return type! I'm going to convert this to a discussion as there's no clear way to move forward yet, and the demand is also not yet clear. |
Beta Was this translation helpful? Give feedback.
-
I would like to propose deprecating
result.partition
from the standard library, and superseding it with a more genericlist.partition_map
function.My reasoning for suggesting this is:
result.partition
is only useful forList(Result(a, e))
. It does not accept any other type, nor does it accept a callback to easily extend its usefulness beyond the simple case of collecting successes and failures into separate lists.list.partition_map
(if implemented as suggested) is generic overList(a)
, and does accept a callback to extend its behaviour without a separate call tolist.map
.result.partition
being located in theresult
module rather than thelist
module makes it harder to find for those who might find use in it outside of the case mentioned above.list.partition_map
would provide identical behaviour toresult.partition
when passed an identity function (e.g.fn(a) { a }
) as callback.result.partition
returns the lists in reverse order compared to the input list, requiring a separate call tolist.reverse
to correct the order. Regarding this,list.partition_map
would provide consistent behaviour to the otherlist
module functions.For ease of reference, this is the current implementation of
result.partition
:And this is the implementation of
list.partition_map
that I propose:I've written this to be similar to
list.partition
andlist.filter_map
.As this is my first issue on this repository, please let me know if there are any improvements I can make to this comment.
Beta Was this translation helpful? Give feedback.
All reactions