-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add Enum.sum_by/2 #12885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Enum.sum_by/2 #12885
Conversation
My biggest concern with |
I understand this concern, my thinking was that filtering is still possible by returning the noop value 0 or 1. Maybe we can explicitly provide an example doing so in the docs ? An explicit alternative could be |
I've been thinking about this one, and It would be consistent with map_join([t], (t -> String.t())) :: String.t()
map_sum([t], (t -> number())) :: number() Type-wise, max_by([t], (t -> u)) :: t
uniq_by([t], (t -> u)) :: [t] Another benefit of |
I'm personally ok with |
I think it would be good from the perspective of encouraging users to pick efficiency without sacrificing readability. |
Closing for now since the appropriate solution still needs discussion. |
@sabiwara you said closing as it still needs discussion but immediately reopened - accidental double click? 😅 |
Nope, Jose asked me to keep it open 😉 |
Closing this, as we were unable to reach a conclusion. |
Learning Elixir right now. The absence of sum/2 is quite sad. We are doing: item[:quantity_by_size]
|> Enum.map(fn {_k, v} -> v end)
|> Enum.sum and the exercise says
Here is what I did: defp sum_by(enumerable, fun) do
enumerable |> Enum.reduce(0, fn element, acc -> fun.(element) + acc end)
end
item[:quantity_by_size]
|> sum_by(fn {_k, v} -> v end) So are we doomed to rewrite a sum/2 with a low level tool like Please let us write item[:quantity_by_size]
|> Enum.sum(fn {_k, v} -> v end) |
I have revisited my previous argument. And I found a flaw in it:
We already have So if everyone agrees, I am fine with adding this as EDIT: I believe @sabiwara mentioned this earlier as well. |
@josevalim after thinking about this for a while, I was more leaning towards
OTOH, for a callback of type I am fine either way, |
Co-authored-by: Wojtek Mach <[email protected]>
Co-authored-by: Łukasz Jan Niemier <[email protected]>
Proposal draft to include
Enum.sum/2
.If we agree on the signature, will send a similar PR for
Enum.product/2
.Side note:
might also bench specific implementations for lists to see if it's worth it.Edit: done, shows a 40% increase. Not bad, will update accordingly.