Feature #22297: Module#method_defined? should have an `include_private` argument

• Status: Closed
• Assignee: -
• Target version: -

[byroot (Jean Boussier)]

Active Record generate methods during boot time based on the database schema. However before defining every one of these methods it first check if it
already exists, regardless of visibility, as to avoid overwriting use defined code.

As a result, the following pattern is in a few hot spots:

if method_defined?(name) || private_method_defined?(name)
  # ...
end

On our Rails monolith, private_method_defined? account for 1% of boot time, that's not massive, but it could very easily be eliminated if there was a way to check for a method existence regardless of its visibility.

Proposal

Currently Module#method_defined? signature is:

method_defined?(symbol, inherit=true)

I think we could make it:

method_defined?(symbol, inherit=true, include_all=false)

[matz (Yukihiro Matsumoto)]

Accepted. method_defined?(name, inherit = true, include_all = false) looks good.

I considered adding a new method instead, but the only difference would be visibility, and I couldn't find a good name for it. An optional positional argument is simpler, and it is consistent with respond_to?(name, include_all).

The argument should be added only to method_defined?. public_method_defined?, private_method_defined? and protected_method_defined? already specify visibility by name.

Matz.

[byroot (Jean Boussier)]

• Status changed from Open to Closed

Applied in changeset git|fad4751ae24b89b3a2841edbdca0877e80248d96.

---

Module#method_defined? optional arguments to also match private method

[Feature #22297]

When generating code dynamically, you often need to check whether
the method already exists, regardless of visibility.

Currently it requires two method calls and two method lookups:

mod.method_defined?(name) || mod.private_method_defined?(name)

This is both inconvenient and wastefull in hotspots.

You can now match all methods regardless of visibility with
a single lookup:

mod.method_defined?(name, true, true)

[matheusrich (Matheus Richard)]

I dislike positional boolean args (see #17938). I'm not aware of the performance impact, but would it be possible to use a kwarg here?

[byroot (Jean Boussier)]

@matheusrich (Matheus Richard) I considered a keyword argument, but went with positional for consistency.

This isn't yet in a released version, so feel free to come forward with a proposal to make these keyword arguments before the final 4.1 release.

As for performance, yes, keyword arguments have some extra cost when parsed from C, but in this case we can probably move the argument parsing to Ruby, so probably negligible.