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

Fix memoization with Hash-like objects as parameters #25

Merged

Conversation

AlexWayfer
Copy link
Contributor

After adding **kwargs due to warnings in Ruby 2.7,
Hash-like parameters (with #to_hash method) convert to Hash
while passed in dynamically created by Memery method.
For example, instances of Sequel::Model.

Found Ruby bug: https://bugs.ruby-lang.org/issues/14909

So, changes will be in Ruby 2.8.

And for now we can:

  1. Use **(;{}) as the additional parameter at method call, what is ugly.
  2. Define memoized methods with eval and exactly expected arguments,
    what is dangerous (eval is evil).
  3. Suppress warnings about keyword arguments until here are no failing tests.

First, I've chose the 3th.

But then Jeremy Evans suggest to use ruby2_keywords. So, here we're!

Additional line of code offensed RuboCop (method length),
so I was forced to refactor method definition.

lib/memery.rb Outdated Show resolved Hide resolved
@AlexWayfer AlexWayfer force-pushed the fix_memoization_with_hash-like_objects branch from 9c35df4 to fe1e661 Compare February 10, 2020 12:43
After adding `**kwargs` due to warnings in Ruby 2.7,
Hash-like parameters (with `#to_hash` method) convert to Hash
while passed in dynamically created by Memery method.
For example, instances of `Sequel::Model`.

Found Ruby bug: https://bugs.ruby-lang.org/issues/14909

So, changes will be in Ruby 2.8.

And for now we can:

1.  Use `**(;{})` as the additional parameter at method call, what is ugly.
2.  Define memoized methods with `eval` and exactly expected arguments,
    what is dangerous (eval is evil).
3.  Suppress warnings about keyword arguments until here are no failing tests.

First, I've chose the 3th.

But then Jeremy Evans suggest to use `ruby2_keywords`. So, here we're!

Additional line of code offensed RuboCop (method length),
so I was forced to refactor method definition.
@AlexWayfer AlexWayfer force-pushed the fix_memoization_with_hash-like_objects branch from fe1e661 to 903be49 Compare February 10, 2020 12:46
@tycooon tycooon merged commit 0c65d19 into tycooon:master Feb 10, 2020
@AlexWayfer AlexWayfer deleted the fix_memoization_with_hash-like_objects branch February 10, 2020 12:58
@tycooon
Copy link
Owner

tycooon commented Feb 10, 2020

Thanks for the patch!

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

Successfully merging this pull request may close these issues.

2 participants