Memoizing methods with implicit blocks may surprise user

sferik commented 10 years ago

Consider the following code:

class Foo

  def bar
    yield
  end
  memoize :bar

  def baz
    yield if block_given?
  end
  memoize :baz

end

Currently, if a user memoizes a method with an explicit block (or any explicit parameters), they will receive an error at load time (Memoizable::MethodBuilder::InvalidArityError). However, if a method yields to an implicit block, users will receive a LocalJumpError (in the case of bar) or—worse—no error at all (in the case of baz). This behavior may surprise users, who don’t expect memoize to modify the behavior of their methods.

This seems bad but I can’t think of a good solution so I’m opening this issue for discussion. Curious to hear your thoughts…

dkubb commented 10 years ago

I would probably raise an exception if block_given? is true. I had meant to do this at some point but I hadn't gotten around to it.

It's one of my requirements in https://gist.github.com/dkubb/7723595 so I suppose I should make the change sooner rather than later.

mbj commented 10 years ago

+1 for raise exceptions on block given!

sferik commented 10 years ago

Ah, yes. That sounds good to me.

dkubb commented 10 years ago

@sferik @mbj I created PR #8 that makes this change.

dkubb / memoizable

Memoizing methods with implicit blocks may surprise user #6