Project

General

Profile

Actions

Feature #17330

open

Object#non

Added by zverok (Victor Shepelev) almost 4 years ago. Updated about 2 years ago.

Status:
Open
Assignee:
-
Target version:
-
[ruby-core:100897]

Description

(As always "with core" method proposals, I don't expect quick success, but hope for a fruitful discussion)

Reasons:

Ruby always tried to be very chainability-friendly. Recently, with introduction of .then and =>, even more so. But one pattern that frequently emerges and doesn't have good idiomatic expression: calculate something, and if it is not a "good" value, return nil (or provide default value with ||). There are currently two partial solutions:

  1. nonzero? in Ruby core (frequently mocked for "inadequate" behavior, as it is looking like predicate method, but instead of true/false returns an original value or nil)
  2. ActiveSupport Object#presence, which also returns an original value or nil if it is not "present" (e.g. nil or empty? in AS-speak)

Both of them prove themselves quite useful in some domains, but they are targeting only those particular domains, look unlike each other, and can be confusing.

Proposal:

Method Object#non (or Kernel#non), which receives a block, calls it with receiver and returns nil (if block matched) or receiver otherwise.

Prototype implementation:
class Object
  def non
    self unless yield(self)
  end
end
Usage examples:
  1. With number:

    limit = calculate.some.limit
    limit.zero? ? DEFAULT_LIMIT : limit
    # or, with nonzero?
    calculate.some.limit.nonzero? || DEFAULT_LIMIT
    # with non:
    calculate.some.limit.non(&:zero?) || DEFAULT_LIMIT
    # ^ Note here, how, unlike `nonzero?`, we see predicate-y ?, but it is INSIDE the `non()` and less confusing
    
  2. With string:

    name = params[:name] if params[:name] && !params[:name].empty?
    # or, with ActiveSupport:
    name = params[:name].presence
    # with non:
    name = params[:name]&.non(&:empty?)
    
  3. More complicated example

    action = payload.dig('action', 'type')
    return if PROHIBITED_ACTIONS.include?(action)
    send("do_#{action}")
    # with non & then:
    payload.dig('action', 'type')
      .non { |action| PROHIBITED_ACTIONS.include?(action) }
      &.then { |action| send("do_#{action}") }
    

Basically, the proposal is a "chainable guard clause" that allows to "chain"ify and DRYify code like:

value = fetch_something
return value unless value.with_problems?
# which turns into
fetch_something.non(&:with_problems?)

# or
value = fetch_something
value = reasonable_default if value.with_problems?
# turns into
value = fetch_something.non(&:with_problems?) || reasonable_default

I believe that this idiom is frequent enough, in combinations like (assorted examples) "read config file but return nil if it is empty/wrong version", "fetch latest invoice, but ignore if it has an unpayable flag", "fetch a list of last user's searches, but if it is empty, provide default search hints" etc.

I believe there is un unreflected need for idiom like this, the need that is demonstrated by the existence of nonzero? and presence.


Related issues 1 (0 open1 closed)

Related to Ruby master - Feature #12075: some container#nonempty?Feedbackmatz (Yukihiro Matsumoto)Actions
Actions #1

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

Actions #3

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

  • Description updated (diff)
Actions #4

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

  • Description updated (diff)

Updated by sawa (Tsuyoshi Sawada) almost 4 years ago

I think the proposed feature would be useful, but I feel that your focus on use cases with a negative predicate is artificial. Positive predicates should have as many corresponding use cases. And since negation is always one step more complex than its positive counterpart, you should first (or simultaneously) propose the positive version, say Object#oui:

calculate.some.limit.oui(&:nonzero?) || DEFAULT_LIMIT

params[:name]&.oui{ _1.empty?.! }

payload.dig('action', 'type').oui{ PROHIBITED_ACTIONS.include?(_1).! }

I think such feature has actually been proposed in the past.

Furthermore, I suggest you may also propose the method(s) to take a variable numbers of arguments to match against:

payload.dig('action', 'type').non(*PROHIBITED_ACTIONS)

And by "match", I think that using the predicate === would be more useful than ==:

"foo".non(/\AError: /, /\AOops, /) # => "foo"
"Oops, something went wrong.".non(/\AError: /, /\AOops, /) # => nil

Updated by akr (Akira Tanaka) almost 4 years ago

I prefer "not" method than this "non" method.

x.empty?.not

Although x.empty?.! is possible now, ! method is not very readable.

Updated by matz (Yukihiro Matsumoto) almost 4 years ago

I don't see the non method make code more readable by glancing at the examples.
Can you elaborate on the benefit of the proposal?

Matz.

Updated by zverok (Victor Shepelev) almost 4 years ago

@matz (Yukihiro Matsumoto) Thinking a bit more about it, what I frequently lack is a "singular reject": "drop this value if it doesn't match expectations".

# HTTP-get something, don't return result unless successful:
response = Faraday.get(url)
return response unless response.code >= 400
# or worse:
unless response.code >= 400
  return response.body
end

# With non
return Faraday.get(url).non { |r| r.code >= 400 }
# Note how little changes necessary to say "successful response's body"
return Faraday.get(url).non { |r| r.code >= 400 }&.body

Though, it seems for me when explaining that one needs a symmetric pair of "singular select"/"singular reject", because particular Faraday examples maybe better written as

return Faraday.get(url).when(&:successful?)
# Note how little changes necessary to say "successful response's body"
return Faraday.get(url).when(&:successful?)&.body

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

zverok (Victor Shepelev) wrote in #note-8:

return Faraday.get(url).when(&:successful?)
# Note how little changes necessary to say "successful response's body"
return Faraday.get(url).when(&:successful?)&.body

This seems readable enough and more flexible.

return Faraday.get(url).then {_1.body if _1.successful?}

Or, make successful? to return self or nil, then

return Faraday.get(url).successful?&.body

Updated by zverok (Victor Shepelev) almost 4 years ago

@nobu (Nobuyoshi Nakada)

This seems readable enough and more flexible.

return Faraday.get(url).then {_1.body if _1.successful?}

I argue that the idiom (return something/continue with something, conditionally) is so frequent, that it should be more atomic.

Or, make successful? to return self or nil

That's what I started this ticket with!

  • This is not natural for Ruby: nonzero? is frequently mocked and generally considered a "design error"; Rails' presence has kind of the same vibe
  • This requires me to change Faraday (which, to be clear, I have no relation to), and then go and change every library which causes the same idiom :)
  • This is solving very particular case (just like nonzero? and presence), I am talking about generic.

Here's one more example:

Faraday.get(url).body.then { JSON.parse(_1, symbolize_names: true) }.non { _1.key?('error') }&.dig('user', 'status')

Updated by Dan0042 (Daniel DeLorme) almost 4 years ago

FWIW, +1 from me

At first I thought the only uses were non(&:zero?) and non(&:empty?) which, while I find very elegant, are not enough to justify adding a method to Kernel. But I think zverok has shown enough other uses to make a convincing case.

There might be something to say about how this provides an automatic complement to any predicate method:

if user.subscription.non(&:expired?)     # is more grammatically correct (English-wise)
if !user.subscription.expired?           # than the traditional negation
if user.subscription.expired?.!          # or akr style

if user.subscription&.non(&:expired?)    # particularly useful in chain
if s = user.subscription and !s.expired?           # the alternatives
if user.subscription&.then{ |s| s if !s.expired? } # are not very
if user.subscription&.then{ _1 if !_1.expired? }   # attractive (IMO)

Overall this "singular reject/select" goes in the same direction as tap and then, so if those two are considered improvements to ruby, I think non and when would be the same kind of improvement.

Can we also consider non(:empty?) in the same vein as inject, or is that an anti-pattern?

Actions #12

Updated by zverok (Victor Shepelev) about 2 years ago

  • Description updated (diff)

Updated by byroot (Jean Boussier) about 2 years ago

I also think not would be a better name for it:

class Object
  def not
    self unless yield self
  end
end

Something that comes up frequently if to check wether an argument is either nil or empty (e.g. https://twitter.com/_ko1/status/1555424953193070593), and safe navigation can't be used because there is no inverse of empty?:

def do_something(arg)
  if arg && !arg.empty?
    # ... snip
  end
end

Which often lead users to incorrectly use any?:

  if arg&.any?
    # ... snip
  end

With this proposal it could be written as:

  if arg&.not(&:empty?)
    # ... snip
  end

Which isn't much shorter, but flows much better IMHO.

Updated by Dan0042 (Daniel DeLorme) about 2 years ago

byroot (Jean Boussier) wrote in #note-13:

I also think not would be a better name for it

Since there is already a not operator, I would expect a not method to have the same semantics and return either true or false. So since we're talking about a method that returns self I prefer "non".
42.nonzero? == 42.non(&:zero?)

Very much agree with the other points.

Actions #15

Updated by mame (Yusuke Endoh) about 2 years ago

params[:name]&.non(&:empty?)

Frankly speaking, this above is dead hard to read and write for me. Do you all really want this?

return Faraday.get(url).non { |r| r.code >= 400 }&.body

This is confusing to me whether to return a body when r.code >= 400 or when r.code < 400.

r = Faraday.get(url)
return r.code < 400 ? r.body : nil

I believe this plain old style is much clearer.

Updated by zverok (Victor Shepelev) about 2 years ago

@mame (Yusuke Endoh) One thing that is really frequent in Rails codebases is code like this:

params.presence || DEFAUT_PARAMS
# or, quoting from above, what I write as...
params[:name]&.non(&:empty?)
# in Rails would be...
params[:name].presence

(not only params-related, it is just an example)

It relies on Rails-y (and PHP-y, but not Ruby-ish) concept of "every empty object is not present". Its existence and popularity is notable. But it is
a) very particular (only empty/falsy objects), and at the same time
b) very imprecise (empty AND falsy objects, so by the .presence you can't tell whether nil could be here or not, or false could be etc.)

I am trying to generalize it to be suitable for the chainable style of computations. The existence of nonzero? highlights that Ruby core devs also tried to address the problem.

I find that frequently existence of "chainable-friendly" API inspires the design of other APIs, say, Faraday example can be simplified with

Faraday.get(url).non(&:error?)&.body

Which spells as a direct telling what's happening: "get response from URL, [then, it it is] non-error, get the response body"

Updated by mame (Yusuke Endoh) about 2 years ago

I understand the logic, but the result of your generalization, &.non(&:empty?), is hard to read anyway. @matz (Yukihiro Matsumoto) says it too in #note-7. The short idiom includes a branch (&.), negation (non), and implicit Symbol#to_proc hack. It is very brain-intensive, and reminiscent of Perl.

I guess the generalization is an impossible idea. I like the specific container#nonempty? in #12075 (except its name). If we were to introduce something even slightly generalized to this problem, x.empty?.not that @akr (Akira Tanaka) proposed in #note-6 would be a reasonable compromise, IMO.

The existence of nonzero? highlights that Ruby core devs also tried to address the problem.

The birth of nonzero? is a bit more particular, and it is considered no longer needed in modern times. I wrote its history in https://bugs.ruby-lang.org/issues/9123#change-43083 . It is not so related to this topic, but you may be interested.

Updated by Dan0042 (Daniel DeLorme) about 2 years ago

mame (Yusuke Endoh) wrote in #note-15:

params[:name]&.non(&:empty?)

Frankly speaking, this above is dead hard to read and write for me. Do you all really want this?

Personally speaking, yes, the above is clear to me, and reads easily, much more than the alternatives.

return Faraday.get(url).non { |r| r.code >= 400 }&.body

On the other hand I also find this one hard to read.
But it's all up to the programmer to write easy-to-read code. This is one of the tools that could help. Or hinder depending on how it's used.
"The possibility to make code cryptic itself should not be the reason to withdraw a feature." -Matz

Updated by ujihisa (Tatsuhiro Ujihisa) about 2 years ago

non() and oui()

Since what these 2 methods do aren't trivial from the names, how's the following 2-words names instead?

itself_if
itself_unless

These return either the object itself or nil, so it makes sense to have the method names what to return.
We are already familiar with the concept of itself/if/unless, so these names should be self descriptive too.

Also for example non is shorter than nonzero? but it doesn't seem to be used more widely than nonzero? in general.


I wonder if itself_unless is actually useful rather than confusing.
You can easily rewrite itself_unless with itself_if with !, such as x.itself_if { !_1.pred? } which is pretty straightforward.

Inspired by the fact that there's Array#delete_if but there's no Array#delete_unless. https://docs.ruby-lang.org/ja/latest/class/Array.html#I_DELETE_IF

Updated by sawa (Tsuyoshi Sawada) about 2 years ago

ujihisa (Tatsuhiro Ujihisa) wrote in #note-19:
[H]ow's the following 2-words names instead?

itself_if
itself_unless

I agree it is more descriptive and may be better. Or, perhaps, we can simply have Object#if, Object#unless.

calculate.some.limit.if(&:nonzero?) || DEFAULT_LIMIT
params[:name]&.unless(&:empty?)

I wonder if itself_unless is actually useful rather than confusing.
You can easily rewrite itself_unless with itself_if with !, such as x.itself_if { !_1.pred? } which is pretty straightforward.

I agree.

Updated by zverok (Victor Shepelev) about 2 years ago

TBH, I believe that core method names should be short, recognizable and elegant in context in the first place, not explaining what they do in multiple words.
We are thinking about "learn once, use 1000 times" here first, not only about a period of introduction of the new method.

It is the same as yield_self, which kinda "said what it did" but looked ugly and over-explaining for everyday use and was promptly renamed to then.
The same is related to the idea of itself_if/itself_unless, I believe (it is like using each_returning_result instead of map).

Just #if and #unless look tempting, though.

Updated by hmdne (hmdne -) about 2 years ago

I had another idea regarding a #non method that I haven't voiced before, but may result in cleaner code overall:

class Negator < ::BasicObject
  def initialize(obj)
    @object = obj
  end

  def method_missing(...)
    !@object.__send__(...) && @object
  end

  def respond_to_missing?(...)
    @object.respond_to?(...)
  end
end

class Object
  def non
    Negator.new(self)
  end
end

[].non.empty? # => false
[123].non.empty? # => [123]

Updated by Dan0042 (Daniel DeLorme) about 2 years ago

ujihisa (Tatsuhiro Ujihisa) wrote in #note-19:

itself_if
itself_unless

zverok (Victor Shepelev) wrote in #note-21:

Just #if and #unless look tempting, though.

Interesting suggestion. Let's look at the previous "non" examples rewritten with "itself_unless" and "unless"

calculate.some.limit.non(&:zero?) || DEFAULT_LIMIT
name = params[:name]&.non(&:empty?)
payload.dig('action', 'type').non { |action| PROHIBITED_ACTIONS.include?(action) }
fetch_something.non(&:with_problems?)
return Faraday.get(url).non { |r| r.code >= 400 }&.body
if user.subscription&.non(&:expired?) 

calculate.some.limit.itself_unless(&:zero?) || DEFAULT_LIMIT
name = params[:name]&.itself_unless(&:empty?)
payload.dig('action', 'type').itself_unless { |action| PROHIBITED_ACTIONS.include?(action) }
fetch_something.itself_unless(&:with_problems?)
return Faraday.get(url).itself_unless { |r| r.code >= 400 }&.body
if user.subscription&.itself_unless(&:expired?) 

calculate.some.limit.unless(&:zero?) || DEFAULT_LIMIT
name = params[:name]&.unless(&:empty?)
payload.dig('action', 'type').unless { |action| PROHIBITED_ACTIONS.include?(action) }
fetch_something.unless(&:with_problems?)
return Faraday.get(url).unless { |r| r.code >= 400 }&.body
if user.subscription&.unless(&:expired?) 

YMMV, but to my eye it looks like "non" is more expressive when used with a single predicate method, and "itself_unless" is more readable for longer blocks. And "unless" is remarkably readable in both situations.

I wonder if itself_unless is actually useful rather than confusing.
You can easily rewrite itself_unless with itself_if with !, such as x.itself_if { !_1.pred? } which is pretty straightforward.

I think it's more useful than itself_if. In many cases the negation itself is what makes this useful, otherwise you would just use the predicate directly.

if user.subscription&.active?              # if subscription has "active?" there is no need here
if user.subscription&.itself_if(&:active?) # to use "oui" or "itself_if"

if user.subscription&.non(&:expired?)           # if subscription only has "expired?" it needs to be negated,
if user.subscription&.itself_if{ !_1.expired? } # which is why we need "non" or a negated "itself_if" or
if user.subscription&.itself_unless(&:expired?) # "itself_unless"

Updated by matz (Yukihiro Matsumoto) about 2 years ago

I still don't think non (or itself_if or itself_unless) gain readability. It may work well in some cases, but may be too easy to be abused.

Matz.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0