-
Notifications
You must be signed in to change notification settings - Fork 13
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
Repair code to avoid "instance variable not initialized" warning #12
Conversation
No issues found by Rubocop. Specs are green. |
I guess you just partly reverted #10, maybe use defined? operator instead? |
I will run a benchmark. I cannot imagine these more time, but let's find out. |
.gitignore
Outdated
|
||
# rspec failure tracking | ||
.rspec_status | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like extra (unneeded) addition.
.gitignore
Outdated
@@ -7,6 +7,8 @@ | |||
/spec/reports/ | |||
/tmp/ | |||
.ruby-version | |||
.ruby-gemset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to do this in separate PR or, at least, commit.
How to get this? Oh, I found: |
Benchmark: # frozen_string_literal: true
require 'bundler/setup'
Bundler.setup
require 'benchmark'
require 'benchmark/ips'
require 'benchmark/memory'
puts '```ruby'
puts File.read(__FILE__).gsub("\t", ' ')
puts '```'
puts
puts '### Output'
puts
puts '```'
require_relative 'lib/memery'
require_relative 'lib/memery_old'
class Foo
class << self
include Memery
include MemeryOld
def base_find(char)
('a'..'k').find { |letter| letter == char }
end
memoize_old def find_old(char)
base_find(char)
end
memoize def find_new(char)
base_find(char)
end
end
end
def memery_old
Foo.find_old('d')
end
def memery_new
Foo.find_new('d')
end
def test
exit unless p (((p memery_old) == (p memery_new)))
end
test
Benchmark.ips do |x|
x.report('memery_old') { memery_old }
x.report('memery_new') { memery_new }
x.compare!
end
Benchmark.memory do |x|
x.report('memery_old') { 100.times { memery_old } }
x.report('memery_new') { 100.times { memery_new } }
x.compare!
end
puts '```' Output
|
I think we should add the |
By the way, actual |
I can try, but it will be without comparison, I guess. Or we need to get the library from Or we can just have an example of bench script (it's ignored for git at my comp now). |
I think that without a comparison is OK – you can always run it on master and your branch and compare manually. |
Done in #14. |
* Add `:ttl` option for `memoize` method (#11) Repeat calling original method after a specified time (in seconds). * Add `--warnings` option for RSpec (#13) * Add benchmark script (#14) Can be executed by `bundle exec ruby benchmark.rb` or `bundle exec rake benchmark`. * Repair code to avoid "instance variable not initialized" warning * Revert previous change
|
@martinstreicher, warnings are gone in the current |
I did check...
The warning persists because line 55 is the same issue I identified previously. |
My Gemfile...
and Gemfile.lock
|
Thanks for the patch! I fixed a rubocop issue and merged. |
Ruby 2.6.2 generates the
instance variable not initialized
warning for a variable in this gem. I moved some code around to avoid the error message.