facets icon indicating copy to clipboard operation
facets copied to clipboard

Enumerable#sum for empty array

Open boomerang opened this issue 8 years ago • 7 comments

I use Enumerable#sum a lot and just upgraded from version 2.9.3 to 3.0 and the behaviour changed.

[].sum should == 0 but i get nil instead.

boomerang avatar Dec 04 '15 13:12 boomerang

Hmm... was my last generalization a bit too much? Have a look at the progression:

  1. https://github.com/rubyworks/facets/commit/ca4aae483f7707b699a3d7f6be6d374528aeee6f
  2. https://github.com/rubyworks/facets/commit/206c1f88c4fb026280cb94a4fdad8383c663fab0
  3. https://github.com/rubyworks/facets/commit/302e0004dd0d3d04ae2fa97209bdfae16e1c9d96

However, there is a certain logic to it b/c now it accommodates any form of summation, i.e. any thing that responds to #+. So for instance [[1],[2],[3]].sum works too (it produces [1,2,3]). The price for this flexibility however is that it will sometimes be necessary to give the identity, e.g. [].sum(0).

What do you think?

trans avatar Dec 05 '15 04:12 trans

I think is´t ok, but change the example documentation in the method :) says that [].sum => 0

I changed to active_support #sum it works as yours did before.

boomerang avatar Dec 07 '15 07:12 boomerang

Ok, thanks. I updated the docs.

Honestly I am on the fence about whether Enumerable#sum should be generic or useful only for numerics.

trans avatar Dec 11 '15 18:12 trans

Ruby 2.4 now includes #sum so be aware that this may cause issues. https://bugs.ruby-lang.org/issues/12217

ioquatix avatar Oct 13 '16 01:10 ioquatix

Thanks. This is a bit unfortunate b/c I really believe the more generalized form is better. Yes, it means passing an argument, e.g. sum(0), for the common case, but it makes you think about what you are doing, e.g. if summing floats. Ruby's version doesn't do that. Boo hiss! And b/c Ruby's is defined on Array instead of Enumerable it override Facets' method in the common cases. Not sure if Matz if right about this choice, it can go either way, it makes sense for some Enumerable classes and not others. In any case Facets will probably have to override Ruby's method to get the better behavior, but if so it has to be done in a compatible way, and that means parameterless sum has to return 0. ActiveSupport took an interesting approach and checked the final return value, if it is nil then it returns 0 instead. Maybe we can do that to, and with any luck there won't be any edge cases (i.e. Facets' sum will be a full superset of Ruby's). If not then the only choice is either convince Matz to change it in Ruby (hard to do) or rename the Facets method -- but to what?

trans avatar Oct 26 '16 15:10 trans

Ruby does have a few weird things in it. I'd suggest that we file a bug report on Ruby.

To be honest, sum is a bit of a stupid method. It would even make more sense [].sum || "empty" but even this is not possible if as you say sum returns 0.

ioquatix avatar Dec 22 '16 01:12 ioquatix

Ah, found this: https://bugs.ruby-lang.org/issues/12902 could be a good idea for us to chime in.

ioquatix avatar Dec 22 '16 01:12 ioquatix