Fixed a bug that overwrote existing self.inherited method definitions.

[alpaca-tc]

After the #324 PR is merged, I will publish this PR.

Before merging:

• Copy the table printed at the end of the latest benchmark results into the README.md and update this PR
• If this change merits an update to CHANGELOG.md, add an entry following Keep a Changelog guidelines with semantic versioning

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c848377) 100.00% compared to head (98bb62b) 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #326   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          183       190    +7     
  Branches        88        90    +2     
=========================================
+ Hits           183       190    +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JacobEvelyn]

I know this is how we defined it in the module_eval, but I'm wondering if there's a reason to keep the super before the create_memo_wise_state! here but after it above. If either works, I don't care which we stick with but think it would be nice to be consistent.

[JacobEvelyn]

Thanks for drafting this, @alpaca-tc ! I'm looking forward to merging this.

I'm also wondering if there's a way to do something similar to solve #321—I can't seem to get something working to address that issue but I have the feeling I'm missing something obvious. Any ideas?

[alpaca-tc]

@JacobEvelyn I think that we can either use prepend or prepend #initialize when .included.
Alternatively, we could just call super in #initialize and improve the error message.

undefined method `fetch' for nil:NilClass (NoMethodError)

is too confusing.

[JacobEvelyn]

Thanks @alpaca-tc ! Here are the new benchmarks to put into the README for this change:

|Method arguments|`Dry::Core`\* (1.0.1)|`Memery` (1.5.0)|
|--|--|--|
|`()` (none)|0.60x|3.58x|
|`(a)`|1.37x|7.41x|
|`(a, b)`|1.20x|6.43x|
|`(a:)`|1.47x|13.60x|
|`(a:, b:)`|1.20x|10.55x|
|`(a, b:)`|1.21x|10.36x|
|`(a, *args)`|0.79x|1.52x|
|`(a:, **kwargs)`|0.77x|2.02x|
|`(a, *args, b:, **kwargs)`|0.69x|1.38x|

|Method arguments|`DDMemoize` (1.0.0)|`Memoist` (0.16.2)|`Memoized` (1.1.1)|`Memoizer` (1.0.3)|
|--|--|--|--|--|
|`()` (none)|22.09x|2.35x|23.72x|2.60x|
|`(a)`|20.98x|14.43x|21.20x|12.20x|
|`(a, b)`|17.45x|12.94x|17.69x|11.13x|
|`(a:)`|29.80x|23.38x|25.17x|21.57x|
|`(a:, b:)`|27.00x|22.26x|23.30x|20.91x|
|`(a, b:)`|25.91x|21.20x|21.88x|19.51x|
|`(a, *args)`|3.07x|2.27x|3.17x|1.95x|
|`(a:, **kwargs)`|2.74x|2.28x|2.51x|2.10x|
|`(a, *args, b:, **kwargs)`|2.14x|1.84x|1.95x|1.72x|

[JacobEvelyn]

@JacobEvelyn I think that we can either use prepend or prepend #initialize when .included. Alternatively, we could just call super in #initialize and improve the error message.

undefined method `fetch' for nil:NilClass (NoMethodError)

is too confusing.

I might be misunderstanding your suggestions but I can't seem to get anything working that resolves the tests added in #321 .

[alpaca-tc]

@JacobEvelyn I put it in code because it makes it easier to communicate.

1. Use prepend

May not be useful in this case because it forces the user to use gem.

diff --git a/spec/memo_wise_spec.rb b/spec/memo_wise_spec.rb
index 0fca4ee..c6aed3d 100644
--- a/spec/memo_wise_spec.rb
+++ b/spec/memo_wise_spec.rb
@@ -353,7 +353,7 @@ RSpec.describe MemoWise do
 
         let(:klass_with_initializer) do
           Class.new do
-            include Module1
+            prepend Module1
             def initialize(*); end
           end
         end

2. prepend #initialize when .included

diff --git a/lib/memo_wise.rb b/lib/memo_wise.rb
index 8aa2c38..8955129 100644
--- a/lib/memo_wise.rb
+++ b/lib/memo_wise.rb
@@ -94,6 +94,19 @@ module MemoWise
   end
   private_constant(:CreateMemoWiseStateOnInherited)
 
+  module CreateMemoWiseStateOnIncluded
+    def included(base)
+      return unless base.is_a?(Class) && !base.singleton_class?
+
+      base.prepend(Module.new do
+        def initialize(...)
+          MemoWise::InternalAPI.create_memo_wise_state!(self)
+          super
+        end
+      end)
+    end
+  end
+
   # @private
   #
   # Private setup method, called automatically by `prepend MemoWise` in a class.
@@ -176,6 +189,12 @@ module MemoWise
         if klass.is_a?(Class) && !klass.singleton_class?
           klass.singleton_class.prepend(CreateMemoWiseStateOnInherited)
         else
+          if klass.is_a?(Module) && !klass.singleton_class?
+            klass.singleton_class.prepend(CreateMemoWiseStateOnIncluded)
+          elsif klass.is_a?(Module)
+            klass.prepend(CreateMemoWiseStateOnIncluded)
+          end
+
           klass.prepend(CreateMemoWiseStateOnInherited)
         end

3. improve the error message

Improve the error message, since the problem can be solved if the user calls super.
Since the implementation of memo_wise is already too complicated, it would be may nice if it were possible to solve the problem by simply making the error messages easier to understand.

diff --git a/lib/memo_wise.rb b/lib/memo_wise.rb
index 8aa2c38..08673ef 100644
--- a/lib/memo_wise.rb
+++ b/lib/memo_wise.rb
@@ -78,6 +78,12 @@ module MemoWise
     end
   HEREDOC
 
+  class VariableForMemoizationNotInitializedError < StandardError
+    def initialize
+      super("@_memo_wise is not initialized. This error occurs whenever you define `#initialize` and not call super.")
+    end
+  end
+
   module CreateMemoWiseStateOnExtended
     def extended(base)
       MemoWise::InternalAPI.create_memo_wise_state!(base)
@@ -194,7 +200,7 @@ module MemoWise
         when MemoWise::InternalAPI::NONE
           klass.module_eval <<~HEREDOC, __FILE__, __LINE__ + 1
             def #{method_name}
+              MemoWise::InternalAPI.assert_memo_wise_state_initialized!(self)
               @_memo_wise.fetch(:#{method_name}) do
                 @_memo_wise[:#{method_name}] = #{original_memo_wised_name}
               end
@@ -204,7 +210,7 @@ module MemoWise
           key = method.parameters.first.last
           klass.module_eval <<~HEREDOC, __FILE__, __LINE__ + 1
             def #{method_name}(#{MemoWise::InternalAPI.args_str(method)})
+              MemoWise::InternalAPI.assert_memo_wise_state_initialized!(self)
               _memo_wise_hash = (@_memo_wise[:#{method_name}] ||= {})
               _memo_wise_hash.fetch(#{key}) do
                 _memo_wise_hash[#{key}] = #{original_memo_wised_name}(#{MemoWise::InternalAPI.call_str(method)})
@@ -226,7 +232,7 @@ module MemoWise
           end
           klass.module_eval <<~HEREDOC, __FILE__, __LINE__ + 1
             def #{method_name}(#{MemoWise::InternalAPI.args_str(method)})
+              MemoWise::InternalAPI.assert_memo_wise_state_initialized!(self)
               _memo_wise_hash = (@_memo_wise[:#{method_name}] ||= {})
               #{layers.join("\n  ")}
             end
@@ -236,7 +242,7 @@ module MemoWise
           # { method_name: { args => { kwargs => memoized_value } } }
           klass.module_eval <<~HEREDOC, __FILE__, __LINE__ + 1
             def #{method_name}(*args, **kwargs)
+              MemoWise::InternalAPI.assert_memo_wise_state_initialized!(self)
               _memo_wise_hash = (@_memo_wise[:#{method_name}] ||= {})
               _memo_wise_kwargs_hash = _memo_wise_hash.fetch(args) do
                 _memo_wise_hash[args] = {}
@@ -249,7 +255,7 @@ module MemoWise
         else # MemoWise::InternalAPI::SPLAT, MemoWise::InternalAPI::DOUBLE_SPLAT
           klass.module_eval <<~HEREDOC, __FILE__, __LINE__ + 1
             def #{method_name}(#{MemoWise::InternalAPI.args_str(method)})
+              MemoWise::InternalAPI.assert_memo_wise_state_initialized!(self)
               _memo_wise_hash = (@_memo_wise[:#{method_name}] ||= {})
               _memo_wise_key = #{MemoWise::InternalAPI.key_str(method)}
               _memo_wise_hash.fetch(_memo_wise_key) do
@@ -570,7 +576,7 @@ module MemoWise
       raise ArgumentError, "Provided args when method_name = nil" unless args.empty?
       raise ArgumentError, "Provided kwargs when method_name = nil" unless kwargs.empty?
 
+      MemoWise::InternalAPI.assert_memo_wise_state_initialized!(self)
       @_memo_wise.clear
       return
     end
diff --git a/lib/memo_wise/internal_api.rb b/lib/memo_wise/internal_api.rb
index 3a80a8b..d2a98b8 100644
--- a/lib/memo_wise/internal_api.rb
+++ b/lib/memo_wise/internal_api.rb
@@ -2,6 +2,10 @@
 
 module MemoWise
   class InternalAPI
+    def self.assert_memo_wise_state_initialized!(obj)
+      raise VariableForMemoizationNotInitializedError unless obj.instance_variable_defined?(:@_memo_wise)
+    end
+
     # Create initial mutable state to store memoized values if it doesn't
     # already exist
     #

[JacobEvelyn]

Thanks @alpaca-tc ! I'm merging this PR and I also appreciate the other code examples you shared. ❤️

I'll talk with @ms-ati about what path we want to take and then we'll work on getting a new version out with your fixes.