Feature #5673

undef_method probably doesn't need to raise an error

Added by Thomas Sawyer over 2 years ago. Updated over 1 year ago.

[ruby-core:41302]
Status:Feedback
Priority:Normal
Assignee:Yukihiro Matsumoto
Category:-
Target version:next minor

Description

Is there any significant reason for #undef_method to raise an error if the method is already undefined? If no, then change it to just continue on. It can return true/false to relay if was undefined vs already undefined.


Related issues

Related to ruby-trunk - Feature #6241: Module#method_defined? with inherited flag Rejected 04/01/2012

History

#1 Updated by Alexey Muranov over 2 years ago

I can imagine that raising errors in such cases might be meant to discourage excessive metaprogramming.

#2 Updated by Yusuke Endoh about 2 years ago

  • Status changed from Open to Assigned
  • Assignee set to Yukihiro Matsumoto
  • Target version changed from 1.9.4 to 2.0.0

#3 Updated by Yukihiro Matsumoto about 2 years ago

  • Status changed from Assigned to Feedback

I think raising error can catch potential bugs earlier. What is the benefit of ignoring error?

Matz.

#4 Updated by Thomas Sawyer about 2 years ago

B/c often times it's not an error. Cases such as undefining method before redefining new one to suppress warning message of overriden method. Or different versions of a library might get used where one has such method and another does not.

If we need to remove a method from a class/module that may or may not have the method defined, it's less optimal. We either have do something like:

if instancemethods(false).include?(:foo)
undef
method(:foo)
end

Or,

begin
undef_method(:foo)
rescue NameError
end

Both of which entail more overhead and considerations than is really necessary.

On the other hand, if it did not raise an error and returned true/false instead, then it is easy enough for us to handle the error if it truly matters.

success = undef_method(:foo)

raise NameError, "undefined method foo' for class#{self}'" unless success

#undef_method is not something that's used very frequently, so I think code improvements for these cases is worth it.

#5 Updated by Adam Prescott about 2 years ago

What about two methods, undefmethod and undefmethod!, one which returns a
boolean, one which raises?

#6 Updated by Nobuyoshi Nakada about 2 years ago

trans (Thomas Sawyer) wrote:

If we need to remove a method from a class/module that may or may not have the method defined, it's less optimal. We either have do something like:

if instancemethods(false).include?(:foo)
undef
method(:foo)
end

You can use Module#method_defined? in this case.

if methoddefined?(:foo)
undef
method(:foo)
end

#7 Updated by Thomas Sawyer about 2 years ago

@nobu Well, first let me point out that if method #foo is private, then it ain't so simple again. But secondly and really more importantly, this would be because I constantly confuse #undefmethod for #removemethod and vice-versa. Really, the name undef always throws me off and has me thinking it's the opposite of def, but it's not. (Yea, okay another pet-peeve.)

So let me back up to the beginning and substitute #removemethod for where I had been saying #undefmethod.

However, now that both of these methods have been brought up, in truth, I think it is equality applicable to both. And really which functionality is "right", just depends on the developers particular usecase.

So after some thought, I think Adam's proposal might be the best idea. It gives us the option of either functionality as needed.

#8 Updated by Yusuke Endoh over 1 year ago

  • Target version changed from 2.0.0 to next minor

Also available in: Atom PDF