From 5e51bdda59c9ba8e5faf86294e3e431bd45f1830 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 29 Aug 2014 15:32:24 -0700 Subject: We tenderized the wrong method! Object#try already had the yield option, just needed some tenderloving instance_eval to fit the bill --- activesupport/CHANGELOG.md | 13 ++++--------- .../lib/active_support/core_ext/object/blank.rb | 16 ++-------------- activesupport/lib/active_support/core_ext/object/try.rb | 11 ++++++++++- activesupport/test/core_ext/object/blank_test.rb | 10 ---------- activesupport/test/core_ext/object/try_test.rb | 4 ++++ 5 files changed, 20 insertions(+), 34 deletions(-) (limited to 'activesupport') diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 5894dab230..3cd2235aec 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,15 +1,10 @@ -* Added yield to Object#presence, so you can do this: +* Added instance_eval version to Object#try, so you can do this: - project.account.owner.presence { name.first } || 'Nobody' + person.try { name.first } - instead of calling twice (which may incur double SQL calls): + instead of: - project.account.owner ? project.account.owner.name.first || 'Nobody' - - or assigning to local variable: - - owner = project.account.owner - owner ? owner.name.first || 'Nobody' + person.try { |person| person.name.first } *DHH* diff --git a/activesupport/lib/active_support/core_ext/object/blank.rb b/activesupport/lib/active_support/core_ext/object/blank.rb index 99fd22ff56..38e43478df 100644 --- a/activesupport/lib/active_support/core_ext/object/blank.rb +++ b/activesupport/lib/active_support/core_ext/object/blank.rb @@ -39,21 +39,9 @@ class Object # # region = params[:state].presence || params[:country].presence || 'US' # - # You can also use this with a block that will be yielded if the object is present - # and the result of that block will then be returned. The block itself is run against - # the instance you're running #presence on (using instance_eval) - # - # project.account.owner.presence { name.first } || 'Nobody' - # # @return [Object] - def presence(&block) - if present? - if block_given? - instance_eval(&block) - else - self - end - end + def presence + self if present? end end diff --git a/activesupport/lib/active_support/core_ext/object/try.rb b/activesupport/lib/active_support/core_ext/object/try.rb index 48190e1e66..31919474ed 100644 --- a/activesupport/lib/active_support/core_ext/object/try.rb +++ b/activesupport/lib/active_support/core_ext/object/try.rb @@ -33,6 +33,11 @@ class Object # ... # end # + # You can also call try with a block without accepting an argument, and the block + # will be instance_eval'ed instead: + # + # @person.try { upcase.truncate(50) } + # # Please also note that +try+ is defined on +Object+, therefore it won't work # with instances of classes that do not have +Object+ among their ancestors, # like direct subclasses of +BasicObject+. For example, using +try+ with @@ -40,7 +45,11 @@ class Object # delegator itself. def try(*a, &b) if a.empty? && block_given? - yield self + if b.arity.zero? + instance_eval(&b) + else + yield self + end else public_send(*a, &b) if respond_to?(a.first) end diff --git a/activesupport/test/core_ext/object/blank_test.rb b/activesupport/test/core_ext/object/blank_test.rb index 4deac4b0aa..d6c69bd582 100644 --- a/activesupport/test/core_ext/object/blank_test.rb +++ b/activesupport/test/core_ext/object/blank_test.rb @@ -28,14 +28,4 @@ class BlankTest < ActiveSupport::TestCase BLANK.each { |v| assert_equal false, v.present?, "#{v.inspect} should not be present" } NOT.each { |v| assert_equal true, v.present?, "#{v.inspect} should be present" } end - - 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_with_a_block - assert_equal "THIS WAS TENDERLOVE'S IDEA", "this was tenderlove's idea".presence { upcase } || "Nobody" - assert_equal "Nobody", nil.presence { upcase } || "Nobody" - end end diff --git a/activesupport/test/core_ext/object/try_test.rb b/activesupport/test/core_ext/object/try_test.rb index 8b754ced53..225c20fa36 100644 --- a/activesupport/test/core_ext/object/try_test.rb +++ b/activesupport/test/core_ext/object/try_test.rb @@ -65,6 +65,10 @@ class ObjectTryTest < ActiveSupport::TestCase assert_equal false, ran end + def test_try_with_instance_eval_block + assert_equal @string.reverse, @string.try { reverse } + end + def test_try_with_private_method_bang klass = Class.new do private -- cgit v1.2.3