Project

General

Profile

Actions

Feature #5673

closed

undef_method probably doesn't need to raise an error

Added by trans (Thomas Sawyer) over 12 years ago. Updated about 6 years ago.

Status:
Feedback
Target version:
-
[ruby-core:41302]

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 1 (0 open1 closed)

Related to Ruby master - Feature #6241: Module#method_defined? with inherited flagRejectedmatz (Yukihiro Matsumoto)04/01/2012Actions

Updated by alexeymuranov (Alexey Muranov) over 12 years ago

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

Updated by mame (Yusuke Endoh) almost 12 years ago

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

Updated by matz (Yukihiro Matsumoto) almost 12 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.

Updated by trans (Thomas Sawyer) almost 12 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 instance_methods(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.

Updated by aprescott (Adam Prescott) almost 12 years ago

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

Updated by nobu (Nobuyoshi Nakada) almost 12 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 instance_methods(false).include?(:foo)
undef_method(:foo)
end

You can use Module#method_defined? in this case.

if method_defined?(:foo)
undef_method(:foo)
end

Updated by trans (Thomas Sawyer) almost 12 years ago

@nobu (Nobuyoshi Nakada) 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 #undef_method for #remove_method 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 #remove_method for where I had been saying #undef_method.

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.

Actions #8

Updated by mame (Yusuke Endoh) over 11 years ago

  • Target version changed from 2.0.0 to 2.6
Actions #9

Updated by naruse (Yui NARUSE) about 6 years ago

  • Target version deleted (2.6)
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0