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 for optional args #75

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion lib/memoist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,25 @@ def self.memoist_eval(klass, *args, &block)
end

def self.extract_reload!(method, args)
if args.length == method.arity.abs + 1 && (args.last == true || args.last == :reload)
# Arity is negative for methods with optional/named args
# We don't want to extract out the last argument in this case.
# Also, if the :memoist_reload flag is passed, we use that
# irrespective of the arity.
if args.length > 0 && args.last.is_a?(Hash)
named_args_hash = args.last
reload = named_args_hash.delete :memoist_reload
# If no other named args were passed, that means memoist_reload is an
# extra arg that we need to get rid of; otherwise Ruby will complain
# about argument number mismatch.
if named_args_hash.empty?
args.delete(named_args_hash)
end
return reload
end
return false if method.arity.negative?

# Legacy purposes, works only for methods with no optional/named args
if args.length == method.arity + 1 && (args.last == true || args.last == :reload)
reload = args.pop
end
reload
Expand Down
24 changes: 17 additions & 7 deletions test/memoist_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ def age

memoize :name, :age

def sleep(hours = 8)
def sleep(hours, mins = 5, seconds: 0)
@counter.call(:sleep)
hours
hours * 3600 + mins * 60 + seconds
end
memoize :sleep

Expand Down Expand Up @@ -241,17 +241,25 @@ def test_memoization

3.times { assert_equal 'Josh', @person.name }
assert_equal 1, @person.name_calls

3.times { assert_equal 'Josh', @person.name(memoist_reload: true) }
assert_equal 4, @person.name_calls
end

def test_memoize_with_optional_arguments
assert_equal 4, @person.sleep(4)
assert_equal 15000, @person.sleep(4, 10)
assert_equal 1, @person.sleep_calls

3.times { assert_equal 4, @person.sleep(4) }
3.times { assert_equal 15000, @person.sleep(4, 10) }
assert_equal 1, @person.sleep_calls

3.times { assert_equal 4, @person.sleep(4, :reload) }
assert_equal 4, @person.sleep_calls
3.times { assert_equal 15000, @person.sleep(4, 10, seconds: 0) }
assert_equal 2, @person.sleep_calls

# There has been exactly one sleep call due to memoization
# Now, with reload, we'll have 3 more calls
3.times { assert_equal 15000, @person.sleep(4, 10, memoist_reload: true) }
assert_equal 5, @person.sleep_calls
end

def test_memoize_with_options_hash
Expand All @@ -261,7 +269,7 @@ def test_memoize_with_options_hash
3.times { assert_equal true, @person.update_attributes(age: 21, name: 'James') }
assert_equal 1, @person.update_attributes_calls

3.times { assert_equal true, @person.update_attributes({ age: 21, name: 'James' }, :reload) }
3.times { assert_equal true, @person.update_attributes({ age: 21, name: 'James' }, memoist_reload: true) }
assert_equal 4, @person.update_attributes_calls
end

Expand Down Expand Up @@ -293,6 +301,8 @@ def test_reloadable
assert_equal 2, @calculator.counter
assert_equal 3, @calculator.counter(true)
assert_equal 3, @calculator.counter
assert_equal 4, @calculator.counter(memoist_reload: true)
assert_equal 4, @calculator.counter
end

def test_flush_cache
Expand Down