Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with extract_reload method and optional arguments #61

Open
dskim opened this issue Nov 29, 2016 · 1 comment
Open

Issue with extract_reload method and optional arguments #61

dskim opened this issue Nov 29, 2016 · 1 comment

Comments

@dskim
Copy link

dskim commented Nov 29, 2016

I've just encountered this weird issue where my method is doing something unexpected.
It seems to be related to extract_reload! method of Memoize.

This method can be found on https://github.com/matthewrudy/memoist/blob/master/lib/memoist.rb#L53.

Here is the code in question

if args.length == method.arity.abs + 1 && (args.last == true || args.last == :reload)
  reload = args.pop
end

It removes the last arguments from the args if it is either true or `:reload which seemingly to support the reloading of the method when it is desired.

However, the problem occurs if you have more than one optional arguments

def blah(first_arg = 'something, second_arg = true)
  #do something
end

Memoize will happily remove that second_arg thinking it is the flag for the reload when it is not so. second_arg will always be true in this case because of that behaviour of Memoize.

I can see that this is because Method#arity returns -1 in this case as there is no required arguments.
I think it's unreliable to rely on arity to determine whether you can safely assume the last argument is the optional value meant for Memoize.

Obviously, I can work around this issue, but this would cause all sorts of weird issues for unsuspecting users so it'd be good to have it fixed.
I'd appreciate if anyone has any thoughts on this?

Thanks,

@CapCap
Copy link

CapCap commented Dec 2, 2016

This is a fairly big issue I think- monkey patched in our code to be

  def self.extract_reload!(method, args)
    if args.length == method.arity.abs + 1 && args.last == :reload
      reload = args.pop
    end
    reload
  end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants