Allow memoization for instances whose classes include a module that includes a module prepending MemoWise

[JacobEvelyn]

As noted by @alpaca-tc in this comment, if we add these tests they fail:

diff --git a/spec/memo_wise_spec.rb b/spec/memo_wise_spec.rb
index 6e73d3a..a2355a1 100644
--- a/spec/memo_wise_spec.rb
+++ b/spec/memo_wise_spec.rb
@@ -365,18 +365,32 @@ RSpec.describe MemoWise do
           end
         end
 
+        let(:nested_module_with_initializer) do
+          Module.new do
+            include Module3
+            def initialize(*); end
+          end
+        end
+
         let(:klass_with_module_with_initializer) do
           Class.new do
             include Module3
           end
         end
 
+        let(:klass_with_nested_module_with_initializer) do
+          Class.new do
+            include Module4
+          end
+        end
+
         let(:instance) { klass.new }
 
         before(:each) do
           stub_const("Module1", module1)
           stub_const("Module2", module2)
           stub_const("Module3", module_with_initializer)
+          stub_const("Module4", nested_module_with_initializer)
         end
 
         it "memoizes inherited methods separately" do
@@ -402,6 +416,14 @@ RSpec.describe MemoWise do
           expect(instance.module1_method_counter).to eq(1)
         end
 
+        it "can memoize klass with nested module with initializer" do
+          instance = klass_with_nested_module_with_initializer.new(true)
+          expect { instance.module1_method }.not_to raise_error
+
+          expect(Array.new(4) { instance.module1_method }).to all eq("module1_method")
+          expect(instance.module1_method_counter).to eq(1)
+        end
+
         it "can reset klass with initializer" do
           instance = klass_with_initializer.new(true)
           expect { instance.reset_memo_wise }.not_to raise_error
@@ -411,6 +433,11 @@ RSpec.describe MemoWise do
           instance = klass_with_module_with_initializer.new(true)
           expect { instance.reset_memo_wise }.not_to raise_error
         end
+
+        it "can reset klass with nested module with initializer" do
+          instance = klass_with_nested_module_with_initializer.new(true)
+          expect { instance.reset_memo_wise }.not_to raise_error
+        end
       end
 
       context "when the class, its superclass, and its module all memoize methods" do

If anyone runs into this issue, please 👍 or comment on this issue to help us prioritize resolving it.

While fixing this, we should see if there are ways to more holistically handle these types of inheritance cases rather than supporting just the cases people file issues for.

[JacobEvelyn]

As we explore implementation paths to handle this and other complex inheritance cases, I want to note @ms-ati noted this idea here which I don't believe has been explored:

I am wondering if we should consider making use of Module#method_added as a callback, and if the method added is #initialize, take some action?

[Galathius]

Hi 👋

Not sure if my way of using this gem is entirely valid, but I ran into an issue when using prepend MemoWise in a base class and then defining memo_wise methods in child classes.

I'm currently migrating a project from memoist (which is no longer maintained) to memo_wise, and this pattern worked fine with memoist. However, with memo_wise, the memoization fails because the module isn’t prepended into the subclass, so the necessary hooks aren’t active when the method is defined.

Here's a minimal example:

class BaseClass
  prepend MemoWise
end

class Child < BaseClass
  memo_wise def slow
    # ...
  end
end

This results in an error at runtime undefined method ``fetch`` for nil:NilClass, which happens because @_memo_wise is not properly initialized in the subclass.

Dynamically prepending MemoWise in the inherited hook of the base class resolves the issue:

class BaseClass
  def self.inherited(subclass)
    super
    subclass.prepend MemoWise
  end
end

Would love to know if this is the intended usage, or if there's a better pattern when working with inheritance.

Full test script

require 'bundler/inline'

gemfile do
source 'https://rubygems.org'

gem 'memo_wise'
gem 'benchmark'
end


class BaseClass
prepend MemoWise
end

class A < BaseClass
def initialize; end

memo_wise def slow
  sleep 2
  puts 'slow result'
end
end

obj = A.new

puts Benchmark.measure { obj.slow }
puts Benchmark.measure { obj.slow }

[ms-ati]

Thank you @Galathius for such a perfectly written issue with a test script.

Please note, before I begin looking into this: the usage of MemoWise is as follows:

class BaseClass
  prepend MemoWise
end

class Child < BaseClass
  def slow
    # ...
  end
  memo_wise :slow
end

Note how the memo_wise call is after the method definition, and passes the method name as a symbol.

I'll take a look

[ms-ati]

@Galathius I am seeing the following:

% ruby test_script.rb
slow result
  0.000222   0.000388   0.000610 (  2.005746)
  0.000013   0.000002   0.000015 (  0.000014)

Given that "slow result" was only printed once, and the second run appears memoized, can you help me to understand the issue you are reporting?

[ms-ati]

# frozen_string_literal: true

require "bundler/inline"

gemfile do
  source "https://rubygems.org"

  gem "memo_wise"
  gem "benchmark"
end

class BaseClass
  prepend MemoWise
end

class A < BaseClass
  def initialize
    super
  end

  def slow
    sleep 2
    puts "slow result"
  end

  memo_wise :slow
end

obj = A.new

puts(Benchmark.measure { obj.slow })
puts(Benchmark.measure { obj.slow })

Running this, I observe the following:

% ruby test_script.rb
slow result
  0.000222   0.000388   0.000610 (  2.005746)
  0.000013   0.000002   0.000015 (  0.000014)

[Galathius]

Hi @ms-ati, thanks for looking into this issue!

Note how the memo_wise call is after the method definition, and passes the method name as a symbol.

Do you think such usage may lead to any issues? AFAIK since ruby 2.1 def method returns symbol of the defined method, so technically memo_wise def slow; end and def slow; end; memo_wise :slow should behave in the same way.

Given that "slow result" was only printed once, and the second run appears memoized, can you help me to understand the issue you are reporting?

Please pay attention that in your example you defined initialize method which calls super, which I think should not be mandatory.

This is quite common pattern – placing boilerplate code (like prepend MemoWise) in a parent class, so letting child classes simply use memo_wise :slow. However, the need to define an initialize method that calls super can feel like an implicit requirement, which might not be immediately obvious to end users.

I've prepared PR for addressing this issue – #392.

[JacobEvelyn]

Note how the memo_wise call is after the method definition, and passes the method name as a symbol.

@ms-ati I agree with @Galathius here—using it like memo_wise def ... is fine and quite a common pattern used with both memo_wise and other similar gems. We should add documentation and tests to make this explicitly supported.

[JacobEvelyn]

Please pay attention that in your example you defined initialize method which calls super, which I think should not be mandatory.

It doesn't appear to be—this code works as I'd expect:

class BaseClass
  prepend MemoWise
end
 
class Child < BaseClass
  memo_wise def slow
    sleep 5
    "done"
  end
end

c = Child.new
c.slow # sleeps, then returns "done"
c.slow # returns "done" immediately

@Galathius is there another minimal reproduction you can provide?

[ms-ati]

Ah! @JacobEvelyn I can reproduce. The error appears when the derived class defines initialize but does not call super inside it:

class Child < BaseClass
  # NOTE: defines initialize but does not call super
  def initialize; end

  memo_wise def slow
    sleep 5
    "done"
  end
end

c = Child.new
c.slow

irb(main):016> c.slow
/Users/msiegel/Development/open_source/memo_wise/lib/memo_wise.rb:174:in 'Child#slow': undefined method 'fetch' for nil (NoMethodError)
	from (irb):16:in '<main>'
	from <internal:kernel>:168:in 'Kernel#loop'
	from /.../.rubies/ruby-3.4.1/lib/ruby/gems/3.4.0/gems/irb-1.14.3/exe/irb:9:in '<top (required)>'
	from /.../.rubies/ruby-3.4.1/bin/irb:25:in 'Kernel#load'
	from /.../.rubies/ruby-3.4.1/bin/irb:25:in '<top (required)>'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/cli/exec.rb:58:in 'Kernel.load'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/cli/exec.rb:58:in 'Bundler::CLI::Exec#kernel_load'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/cli/exec.rb:23:in 'Bundler::CLI::Exec#run'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/cli.rb:484:in 'Bundler::CLI#exec'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/vendor/thor/lib/thor/command.rb:27:in 'Bundler::Thor::Command#run'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in 'Bundler::Thor::Invocation#invoke_command'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/vendor/thor/lib/thor.rb:392:in 'Bundler::Thor.dispatch'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/cli.rb:31:in 'Bundler::CLI.dispatch'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/vendor/thor/lib/thor/base.rb:485:in 'Bundler::Thor::Base::ClassMethods#start'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/lib/bundler/cli.rb:25:in 'Bundler::CLI.start'
	from /.../.gem/ruby/3.4.1/gems/bundler-2.3.8/exe/bundle:48:in 'block in <top (required)>'
	... 4 levels...