Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Don't raise NoMethodError the tried method doesn't exists #1195

Merged
merged 2 commits into from

7 participants

@dmathieu
Collaborator

As said at the time by @rwdaigle in the edge rails blog,

If the method doesn’t exist, or if the target object nil, then nil will be returned without exceptions

That's not the case however.
Only nil returns nil for try. If the method does not exists, a NoMethoderror will be raised.

This pull request changes it.

@dhh
Owner

I've been bitten by this too. Try should definitely ignore NoMethod. Thanks!

@dhh dhh merged commit 073f80e into rails:master
@jake3030 jake3030 referenced this pull request from a commit in jake3030/rails
@miloops miloops Fix: counter_cache should decrement on deleting associated records.
[#1195 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
05f2183
@jake3030 jake3030 referenced this pull request from a commit in jake3030/rails
@NZKoz NZKoz Handle every error that can come out of the Iconv branch by rescuing …
…and returning nil

[#1195 state:committed]
4e4f961
@laserlemon

Could somebody please explain this change?

As I understand it, the purpose of Object#try is to call a method on an object of an expected class, but that could also be nil. In what situation would you want to silently avoid a NoMethodError? If I'm trying a method that doesn't exist in my expected class, I should know about it.

Collaborator

On STI objects where you might have methods defined for some records, but not for others for example.

Owner
Collaborator

Agreed, I think we should revert this.

-1 to this change as well. It's not acceptable to radically change the behavior at this point.

Owner
Collaborator

This commit 587dd7d also need to revert the fix the failing test. Or might need a fix!

587dd7d commit is not having test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  activesupport/lib/active_support/core_ext/object/try.rb
@@ -28,6 +28,8 @@ class Object
def try(*a, &b)
if a.empty? && block_given?
yield self
+ elsif !a.empty? && !respond_to?(a.first)
+ nil
else
__send__(*a, &b)
end
View
8 activesupport/test/core_ext/object_and_class_ext_test.rb
@@ -99,7 +99,13 @@ def setup
def test_nonexisting_method
method = :undefined_method
assert !@string.respond_to?(method)
- assert_raise(NoMethodError) { @string.try(method) }
+ assert_nil @string.try(method)
+ end
+
+ def test_nonexisting_method_with_arguments
+ method = :undefined_method
+ assert !@string.respond_to?(method)
+ assert_nil @string.try(method, 'llo', 'y')
end
def test_valid_method
Something went wrong with that request. Please try again.