Object#presence should take a block #13416

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants

In order to have parallel behavior with methods like #each, presence should
yield to a provided block if the object is present. This is
similar to an anaphoric if in Scheme, and is discussed at length here:

https://github.com/raganwald/homoiconic/blob/master/2012/05/anaphora.md

You can achieve similar functionality by appending try in its block
form to a presence call - but having presence evaluate the block itself
avoids the (confusing) call to try.

@stephenprater stephenprater Object#presence should take a block
In order to have parallel behavior with methods like #each, presence should
yield to a provided block if the object is present.  This is
similar to an anaphoric if in Scheme, and is discussed at length here:

https://github.com/raganwald/homoiconic/blob/master/2012/05/anaphora.m

You can achieve similar functionality by appending `try` in its block
form to a presence call - but having presence evaluate the block itself
avoids the (confusing) call to try.
8a796b0

Can you showcase a real use case for this? I can't really think of anything right now (too late here already :D)

@senny senny commented on the diff Dec 20, 2013

activesupport/test/core_ext/blank_test.rb
@@ -29,4 +29,14 @@ def test_presence
BLANK.each { |v| assert_equal nil, v.presence, "#{v.inspect}.presence should return nil" }
NOT.each { |v| assert_equal v, v.presence, "#{v.inspect}.presence should return self" }
end
+
+ def test_presence_yield
+ BLANK.each do |v|
+ assert_equal nil, v.presence { |it| true }
@senny

senny Dec 20, 2013

Member

use assert_nil

The use case for this is basically the same as an anaphoric if or an assign in a conditional.

For an actual use case:

ordered_products = Product.long_and_complicated_query
if ordered_products.present?
  if sort.present?
    ordered_products = order_products.reorder( by: sort.order_by)
  else
    ordered_products = order_products.reorder
  end
end
ordered_products

# vs. 

Product.long_and_complicated_query.presence do |ordered_products|
  ordered_products.reorder by: sort.presence.order_by
end

That's not ACTUAL production code - but it's the gist of it.

I think sort.presence.order_by would raise if not present in that example, wouldn't it?

In any case, I think we could maybe write your code with something like this:

ordered_products = Product.long_and_complicated_query
ordered_products = order_products.reorder(by: sort.order_by) if sort.present?
ordered_products

In this example I believe you don't need to check for the presence of ordered_products, since that is a relation and would lead to load that relation. Sorry but I still don't see an improvement here.

Thanks!

You're right - it would actually be sort.presence { sort.order_by }

As for the other point - you'd be right - but uhh... confession time.... remember when I said it was the "gist" of the production code? That's because that's not the ActiveRecord reorder method and long_and_complicated_query returns an Array not a relation. I understand that that's a terrible, terrible thing to do - but given that does the use case make any more sense?

FTR I didn't write the terrible method - I'm just trying not to pile additional crap on top of it.

Contributor

avit commented Jan 17, 2014

On the other hand, I could see an argument for the block being called when the value is not present, consistent with hash.fetch(:some_index) { |_, i| default_value_for(i) }.

  • When the value is present, you can just chain on it.
  • When the value is absent, you could pass a block for a default value, and then chain on it.

What is the purpose of having the block option for the first bullet point?

Basically, calling the block is parallel to the behavior of each and other enumerable methods (and try for that matter) - where it's safe to call each on an empty array because they block will never be executed.

Contributor

avit commented Jan 20, 2014

I found these for you, maybe relevant:

https://bugs.ruby-lang.org/issues/6373
https://bugs.ruby-lang.org/issues/6721

In any case, this is a feature request and such discussion happens on the rails-core mailing list. Rails Github issues are meant for bugs: you might get better traction with this question on the mailing list.

Owner

rafaelfranca commented Jan 20, 2014

I still not convinced we need this feature. I also agree with @avit that presence with block would cause confusion if it will run if present or not present.

That said I'm closing this one. Thank you for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment