Uh oh!
There was an error while loading. Please reload this page.
Define DelegateClass methods in separate module - #14
Conversation
| @delegate_dc_obj = obj | ||
| end | ||
| end | ||
| klass.include(Module.new do |
There was a problem hiding this comment.
I'm always a bit annoyed by anonymous modules. I wonder if it would be worth naming this module.
e.g. klass.const_set("DelegationMethods", Module.new).
The benefit is that it makes it much clearer what is going on when introspecting ancestors or method(:foo).owner.
Of course there is the obvious risk of name clash etc, so maybe not worth it?
There was a problem hiding this comment.
I'm always a bit annoyed by anonymous modules.
That's a good point.
Of course there is the obvious risk of name clash etc, so maybe not worth it?
Perhaps a name that deviates from convention would be safe? I've pushed a commit that uses DelegateClass_Methods.
Before this commit, modules included in a `DelegateClass` could not
override delegate methods:
```ruby
Base = Class.new do
def foo
"base"
end
end
Helper = Module.new do
def foo
"helper"
end
end
WithHelper = DelegateClass(Base) { include Helper }
WithHelper.new(Base.new).foo
# => "base"
```
This commit defines delegate methods in a separate module, so other
modules can come before it in the method lookup chain:
```ruby
WithHelper.new(Base.new).foo
# => "helper"
```
Also, because of this change, methods in a `DelegateClass` block will
properly override instead of redefine. Therefore, calling `super` is
faster:
**Benchmark script**
```ruby
# frozen_string_literal: true
require "benchmark/ips"
$LOAD_PATH.prepend(".../delegate/lib")
require "delegate"
Base = Class.new do
def foo
end
end
Overridden = DelegateClass(Base) do
def foo
super
end
end
overridden = Overridden.new(Base.new)
Benchmark.ips do |x|
x.report("super") { overridden.foo }
end
```
**Before**
```
Warming up --------------------------------------
super 75.044k i/100ms
Calculating -------------------------------------
super 759.506k (± 0.8%) i/s - 3.827M in 5.039488s
```
**After**
```
Warming up --------------------------------------
super 184.164k i/100ms
Calculating -------------------------------------
super 1.835M (± 1.0%) i/s - 9.208M in 5.019711s
```
Fixes https://bugs.ruby-lang.org/issues/19079.7caeba1 to
0cb7994Compare
byroot
left a comment
There was a problem hiding this comment.
I like it. Let's add this to the next dev-meeting.
byroot
commented
Oct 25, 2022
nobu
commented
Nov 22, 2022
Why not |
byroot
commented
Nov 22, 2022
@nobu sure you can do that, but you'd have a similar-ish issue with just defining a method in the delegator: In this example It's somewhat assumed that if you create a delegator, you will want to specialize a few methods. |
Before this commit, modules included in a
DelegateClasscould not override delegate methods:This commit defines delegate methods in a separate module, so other modules can come before it in the method lookup chain:
Also, because of this change, methods in a
DelegateClassblock will properly override instead of redefine. Therefore, callingsuperis faster:Benchmark script
Before
After
Fixes https://bugs.ruby-lang.org/issues/19079.