We tenderized the wrong method! Object#try already had the yield option, just needed some tenderloving instance_eval to fit the bill

5 files changed, 20 insertions, 34 deletions

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