Skip to content
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

[nil].sum returns 0 #24796

Closed
mrkn opened this issue Apr 30, 2016 · 16 comments
Closed

[nil].sum returns 0 #24796

mrkn opened this issue Apr 30, 2016 · 16 comments

Comments

@mrkn
Copy link
Contributor

mrkn commented Apr 30, 2016

Is this intentional?

Steps to reproduce

$ pwd
/Users/mrkn/work/rails/master/activesupport
$ git rev-parse --short HEAD
151080a
$ ruby -v -I lib -r active_support/core_ext/enumerable -e 'p [nil].sum'
ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin15]
0

Expected behavior

I don't have the idea what is expected, ideal behavior.
But [nil].sum raises TypeError on Ruby 2.4.

$ ruby -ve '[nil].sum'
ruby 2.4.0dev (2016-04-28 trunk 54810) [x86_64-darwin15]
-e:1:in `+': nil can't be coerced into Fixnum (TypeError)
    from -e:1:in `sum'
    from -e:1:in `<main>'

Actual behavior

Returns 0.

System configuration

Rails version: 151080a

Ruby version: ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin15]

@vipulnsward
Copy link
Member

This is what we rely on and what ruby does

ruby -v 
=> 2.3.0
[nil].inject(:+)
=> nil

Since the end result is nil, we fallback to 0- vipulnsward@5241b97

@mrkn I am not sure if that is intentional though.

@vipulnsward
Copy link
Member

@mrkn I see we behave differently for inject for [nil] and sum. In inject we just return as is, but with sum try to coerce, is that expected?

  ruby git:(trunk)  ./ruby -ve 'puts [nil].inject(:+)'
ruby 2.4.0dev (2016-04-30 trunk 54853) [x86_64-darwin15]
  ruby git:(trunk)  ./ruby -ve 'puts [nil, 1].inject(:+)'
ruby 2.4.0dev (2016-04-30 trunk 54853) [x86_64-darwin15]
-e:1:in `inject': undefined method `+' for nil:NilClass (NoMethodError)
    from -e:1:in `<main>'
  ruby git:(trunk)  ./ruby -ve '[nil].sum'
ruby 2.4.0dev (2016-04-30 trunk 54853) [x86_64-darwin15]
-e:1:in `+': nil can't be coerced into Fixnum (TypeError)
    from -e:1:in `sum'
    from -e:1:in `<main>'
  ruby git:(trunk) 

@mrkn
Copy link
Contributor Author

mrkn commented Apr 30, 2016

In Ruby 2.4, sum isn't identical to inject(:+) while AS's sum is identical to it.
So there some differences between them.

[nil].sum is one of them.

The other difference is the initial value effect.
Ruby 2.4's sum basically assumes numeric elements so the default initial value is 0.
In contrast, AS's sum uses the identity parameter only when the enumerable is empty.
I've posted a pull-request to fix this inconsistency in #24795.

I want to resolve the inconsistency of the behavior of [nil].sum, too.

@mrkn
Copy link
Contributor Author

mrkn commented Apr 30, 2016

I want to resolve the inconsistency of the behavior of [nil].sum, too.

In other words, I want to fix this if it isn't the desired behavior.

@vipulnsward
Copy link
Member

@jeremy
Copy link
Member

jeremy commented May 1, 2016

Great edge case. The identity fallback is meant to catch [].sum. Note that [false].sum is affected in the same way.

@jeremy
Copy link
Member

jeremy commented May 1, 2016

#24795

@jeremy jeremy closed this as completed May 1, 2016
@jeremy jeremy reopened this May 1, 2016
@jeremy
Copy link
Member

jeremy commented May 1, 2016

Oops! I see #24795 does not address this case.

Agree: behavior here is undefined. Expected behavior is [nil].sum == nil and [false].sum == false, matching inject(:+) results.

I think this is buggy, since [single_element].sum == single_element even when !single_element.respond_to?(:+). If there are multiple elements, it would have raised NoMethodError.

@sgrif
Copy link
Contributor

sgrif commented May 1, 2016

Is this buggy? I think it's perfectly reasonable for this to match the behavior of .inject(:+). If Ruby 2.4 provides a sum method, we can defer to that in the future.

@vipulnsward
Copy link
Member

I think we could handle it. Generally, in our Apps, someone would do [].compact.sum and not care about this nil.
But this condition would be only for a single element. (P.S: we have had this behaviour since 2006.)

@Chiether
Copy link

probably related...

before

[].sum #=> 0
[nil].sum #=> 0
[].sum(nil) #=> nil <-- I used this case!
[nil].sum(nil) #=> nil

after:

[].sum #=> 0
[nil].sum #=> 0
[].sum(nil) #=> 0
[nil].sum(nil) #=> 0

That’s life. sigh and replace my code

@junjizhi
Copy link

Confirmed that this is still an issue in Rails 5.2.1

Loading development environment (Rails 5.2.1)
irb(main):001:0> [nil].sum
=> 0

But in ruby 2.5:

$ ruby -v
ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-darwin17]

irb(main):001:0> [nil].sum
Traceback (most recent call last):
        4: from /usr/local/bin/irb:11:in `<main>'
        3: from (irb):1
        2: from (irb):1:in `sum'
        1: from (irb):1:in `+'
TypeError (nil can't be coerced into Integer)

@loganb
Copy link

loganb commented Oct 17, 2018

Just to make sure the variant is covered: [].sum currently throws an exception when it should return 0

@okuramasafumi
Copy link
Contributor

The latest version of Ruby/Rails (5.2.3 and 2.6.3) can both handle [].sum gracefully with returning 0.

@0xYZSR
Copy link

0xYZSR commented Jun 30, 2021

Loading development environment (Rails 5.2.6)
irb(main):001:0> [].sum
Traceback (most recent call last):
        1: from (irb):1
NoMethodError (undefined method `each' for nil:NilClass)

@zzak
Copy link
Member

zzak commented Jul 1, 2021

@blk0 Can you reproduce this in 6.1? Since 5.2 is no longer maintained for bug fixes it's unlikely this will get fixed.

@byroot byroot closed this as completed Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests