Bug #15987
closedLet `exception` option in `Kernel#Complex`, `Kernel#Float`, `Kernel#Integer`, `Kernel#Rational` be falsy vs. truthy
Added by sawa (Tsuyoshi Sawada) over 5 years ago. Updated over 5 years ago.
Description
The exception
option in Kernel#Complex
, Kernel#Float
, Kernel#Integer
, and Kernel#Rational
distinguishes false
vs. other values.
Integer("z", exception: false) #=> nil
Integer("z", exception: nil) #>> ArgumentError: invalid value for Integer(): "z")
But in most other cases where a boolean notion is concerned (for example, the chomp
option in Kernel#gets
), the distinction is between falsy vs. truthy values.
I request the distinction to be falsy vs. truthy. In other words, I would like the value nil
to work on the falsy side rather than the truthy side like this.
Integer("z", exception: false) #=> nil
Integer("z", exception: nil) #=> nil
Updated by sawa (Tsuyoshi Sawada) over 5 years ago
- Subject changed from Let `exception` option in `Kernel#Complex`, `Kernel#Float`, `Kernel#Integer`, `Kernel#Rational` be falsy vs. truthy to Let boolean option (such as `exception` in `Kernel#Complex`, `Kernel#Float`, `Kernel#Integer`, `Kernel#Rational`) be falsy vs. truthy
- Description updated (diff)
Updated by shevegen (Robert A. Heiler) over 5 years ago
I don't have a huge preference either way, but this may be a design decision, so perhaps we
can ask matz either way; also in how strong the concept of "falsy" and "truthy" is in ruby.
I assume there were design considerations for when matz added nil and false and not
considered both to be equivalent (e. g. if some value is nil, you can use that to
distinguish it from true or false).
I think we can find examples where nil being treated as false makes sense, but we may also
consider cases where nil being false does not make a whole lot of sense, e. g. if that
variable would lateron be initialized to become a String or Array. Perhaps that may
have been a reason why you did limit the suggestion to only specific methods, e. g.
chomp in Kernel#gets. For specific methods, it may be that it makes sense to only
distinguish true/false boolean states.
Aside from the conceptual and design consideration, what impact would we be able to
observe if this would be changed? I have not thought through all the given methods
yet. :)
Updated by Eregon (Benoit Daloze) over 5 years ago
I agree for methods taking an :exception keyword argument.
I would consider this a bug to treat nil
as "want exception".
Do you have examples of other methods taking a boolean option and treating nil
the same as true
?
Those should probably be fixed too, but they might be hard to find.
Updated by mame (Yusuke Endoh) over 5 years ago
@sawa (Tsuyoshi Sawada) Could you write a code example in the description? It would be very helpful to make the developer meeting efficiently.
p Integer("z", exception: false) #=> nil
p Integer("z", exception: nil) #=> excepted: nil, actual: invalid value for Integer(): "z" (ArgumentError)
Updated by sawa (Tsuyoshi Sawada) over 5 years ago
- Subject changed from Let boolean option (such as `exception` in `Kernel#Complex`, `Kernel#Float`, `Kernel#Integer`, `Kernel#Rational`) be falsy vs. truthy to Let `exception` option in `Kernel#Complex`, `Kernel#Float`, `Kernel#Integer`, `Kernel#Rational` be falsy vs. truthy
- Description updated (diff)
Updated by matz (Yukihiro Matsumoto) over 5 years ago
- Tracker changed from Feature to Bug
- Backport set to 2.5: UNKNOWN, 2.6: UNKNOWN
It's a bug. It should be fixed.
Matz.
Updated by nobu (Nobuyoshi Nakada) over 5 years ago
Note: The rdoc of Kernel#Integer says:
Passing
nil
raises a TypeError, while passing a String that
does not conform with numeric representation raises an ArgumentError.
and states:
This behavior can be altered by passing
exception: false
,
in this case a not convertible value will returnnil
.
So the current behavior is as documented, at least.
And this seems coming from IO#read_nonblock
and so on.
Updated by sawa (Tsuyoshi Sawada) over 5 years ago
nobu (Nobuyoshi Nakada) wrote:
this seems coming from
IO#read_nonblock
and so on.
I initially had a feeling that I had seen this opposition between false
vs. other values somewhere else, but had forgotten where. Now that nobu has pointed it, I would like to request these related keywords be turned to falsy vs. truthy as well.
Updated by nobu (Nobuyoshi Nakada) over 5 years ago
- Status changed from Open to Closed
Applied in changeset git|3e7d002118a92fad5934e11c75be6768a1476c1b.
Check exception flag as a bool [Bug #15987]
Updated by nobu (Nobuyoshi Nakada) over 5 years ago
It doesn't feel good for me to allow arbitrary values other than true
as truthy.
nil
as falsy is not so weird...
Updated by matz (Yukihiro Matsumoto) over 5 years ago
Options are:
- accept normal truthy/falsey value for boolean (using
RTEST()
) - accept
true
/false
for boolean; exception for everything else
And we should keep consistency.
Matz.
Updated by znz (Kazuhiro NISHIYAMA) over 5 years ago
I think nil
means default behavior.
Integer("") # => ArgumentError
Integer("", exception: nil) # => ArgumentError
Integer("", exception: true) # => ArgumentError
Integer("", exception: false) # => nil
system("") # => nil
system("", exception: nil) # => nil
system("", exception: true) # => Errno::ENOENT
system("", exception: false) # => nil
Updated by sawa (Tsuyoshi Sawada) over 5 years ago
znz (Kazuhiro NISHIYAMA) wrote:
I think
nil
means default behavior.
That does not hold even if you just look at the Kernel
methods:
exit(status=true)
Kernel::exit(status=true)
Process::exit(status=true)
exit!(status=false)
gets(sep=$/ [, getline_args])
rand(max=0)
readline(sep=$/)
readlines(sep=$/)
These methods have default values other than nil
.
Updated by Eregon (Benoit Daloze) over 5 years ago
matz (Yukihiro Matsumoto) wrote:
Options are:
- accept normal truthy/falsey value for boolean (using
RTEST()
)- accept
true
/false
for boolean; exception for everything elseAnd we should keep consistency.
Both sound fine to me.
nobu implemented the second option.
We can try to keep consistency in the future,
but I think we can't easily change existing methods taking a truthy/falsey argument for compatibility.
Updated by nobu (Nobuyoshi Nakada) over 5 years ago
sawa (Tsuyoshi Sawada) wrote:
znz (Kazuhiro NISHIYAMA) wrote:
I think
nil
means default behavior.That does not hold even if you just look at the
Kernel
methods:
As for optional parameters, there are some inconsistencies.
exit(status=true) Kernel::exit(status=true) Process::exit(status=true) exit!(status=false) gets(sep=$/ [, getline_args]) rand(max=0) readline(sep=$/) readlines(sep=$/)
These methods have default values other than
nil
.
And other methods fall back nil
to the default value, e.g., File.open
.
Updated by sawa (Tsuyoshi Sawada) over 5 years ago
nobu (Nobuyoshi Nakada) wrote:
As for optional parameters, there are some inconsistencies.
...
And other methods fall backnil
to the default value, e.g.,File.open
.
I came up with another alternative.
Since this is about suppressing an exception, I think the concept is close to the optional second argument/block of Hash#fetch
. One is keyword argument and the other is positional argument, but that does not mean that the values should behave differently. I think we can imitate it.
That is, when the explicit exception
option is given, then that value should be returned instead of raising an error when the string cannot be parsed correctly. This would be a breaking change, but I do not think it will break much existing code because most use cases so far probably only use the exception
option with the value false
, in which case it currently returns nil
, and my proposal would change that to return false
, but I do not think that is a big deal.
Updated by Eregon (Benoit Daloze) over 5 years ago
This issue is closed, it's probably best to discuss on a new one for different ideas.
sawa (Tsuyoshi Sawada) wrote:
but I do not think that is a big deal.
IMHO it is a big deal, Integer() should return either an Integer
or nil
, but never false
or any other kind of value.
nil
means "absence of value", false
doesn't have the same meaning.
I believe everyone understands the exception: keyword should be given some kind of boolean value, using the value for anything else than deciding whether to raise exceptions would be very surprising.