Project

General

Profile

Bug #15987

Let `exception` option in `Kernel#Complex`, `Kernel#Float`, `Kernel#Integer`, `Kernel#Rational` be falsy vs. truthy

Added by sawa (Tsuyoshi Sawada) 4 months ago. Updated 4 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:93552]

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

Associated revisions

Revision 3e7d0021
Added by nobu (Nobuyoshi Nakada) 4 months ago

Check exception flag as a bool [Bug #15987]

Revision c2723e59
Added by nobu (Nobuyoshi Nakada) 4 months ago

Removed wrong argument in the fallback function [Bug #15987]

Revision f74e23af
Added by nobu (Nobuyoshi Nakada) 4 months ago

Fixed argument in the fallback function [Bug #15987]

Revision 8745fa2f
Added by nobu (Nobuyoshi Nakada) 4 months ago

Default to true when no exception flag [Bug #15987]

History

#1

Updated by sawa (Tsuyoshi Sawada) 4 months ago

  • Description updated (diff)
  • 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

Updated by shevegen (Robert A. Heiler) 4 months 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) 4 months 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) 4 months 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)
#5

Updated by sawa (Tsuyoshi Sawada) 4 months ago

  • Description updated (diff)
#6

Updated by sawa (Tsuyoshi Sawada) 4 months ago

  • Description updated (diff)
  • 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

Updated by matz (Yukihiro Matsumoto) 4 months ago

  • Backport set to 2.5: UNKNOWN, 2.6: UNKNOWN
  • Tracker changed from Feature to Bug

It's a bug. It should be fixed.

Matz.

Updated by nobu (Nobuyoshi Nakada) 4 months 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 return nil.

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) 4 months 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.

#10

Updated by nobu (Nobuyoshi Nakada) 4 months 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) 4 months 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) 4 months 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) 4 months 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) 4 months 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) 4 months ago

matz (Yukihiro Matsumoto) wrote:

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.

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) 4 months 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) 4 months ago

nobu (Nobuyoshi Nakada) wrote:

As for optional parameters, there are some inconsistencies.
...
And other methods fall back nil 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) 4 months 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.

Also available in: Atom PDF