Memoization of singleton methods raises NoMethodError

[sferik]

I was messing around in irb (actually, pry) and felt :confused: when I attempted to memoize a method outside of a class context:

[1] pry(main)> require 'memoizable'
=> true
[2] pry(main)> include Memoizable
=> Object
[3] pry(main)> def foo; end
=> nil
[4] pry(main)> memoize :foo
NoMethodError: undefined method `each' for :foo:Symbol
from /Users/e/.rbenv/versions/1.9.3-p484/lib/ruby/gems/1.9.1/gems/memoizable-0.2.0/lib/memoizable/memory.rb:59:in `set'

This is the line where the error is occurring, which I believe is being called from here.

This error occurs when attempting to memoize singleton methods, defined on an instance of a class:

require 'memoizable'

class Foo
  include Memoizable
end

foo = Foo.new
def foo.bar; end
foo.memoize :bar

To resolve this issue we must first address the following question: Should one be able to memoize singleton methods?

If the answer is “yes”, resolving this issue entails adding that feature.

If the answer is “no”, we can just raise a better error message.

[dkubb]

I don't typically use singleton methods, or anything that modifies the singleton class (see next caveat), due to the negative impact on the method cache, but from an interface point of view I don't have a strong argument against it.

The one exception is that I do sometimes modify the singleton class of a Class or Module instance, but I would generally only do this during "system setup" rather than at "runtime". I use air quotes because these two states aren't enforced by the language, but rather by convention in most ruby systems.

My only argument would be if the implementation was made more complex by supporting the use case.

[dkubb]

@sferik hmm, after looking at this a bit, I think I understand what's going on. The instance has a #memoize method that instead of behaving like the class-level method of the same name, it accepts a Hash where the key is the method name, and caches that in the instance's private cache.

The following does work though:

require 'memoizable'

class Foo
  include Memoizable
end

foo = Foo.new
def foo.bar; 'bar' end

# call memoize on the singleton class
class << foo
  memoize :bar
end

foo.bar.equal?(foo.bar)  # => true

[dkubb]

My first thought was to rename Memoizable::InstanceMethods#memoize to something else, and then make a #memoize method that does class << foo; memoize(*args) end, but I'm not sure if I want to provide an interface for modifying an instance's singleton class.

I don't like the idea of making it too easy to do something that could have such a negative impact on performance. Doing something potentially "dangerous" shouldn't be wrapped up in a pretty interface, it should be a little bit ugly so it's more likely that you should stop and think about the trade-offs before doing it.

However, I would support having a bit of documentation for this specific use case so someone who understands the trade-off can decide whether it makes sense for their specific use case.

@sferik I'm going to mark this as closed for now, but if you want to send a commit or PR with a documentation change I would support it.