evanphx / benchmark-ips

Provides iteration per second benchmarking for Ruby
MIT License
1.72k stars 97 forks source link

Make block arguments optional #27

Closed Arcovion closed 9 years ago

Arcovion commented 9 years ago
Benchmark.ips do |bm|
  bm.report 'Fast' do
    ...
  end

  bm.report 'Slow' do
    ...
  end

  bm.compare!
end

Benchmark.ips do
  report 'Fast' do
    ...
  end

  report 'Slow' do
    ...
  end

  compare!
end

Less typing, easier on the eyes, backwards compatible.

evanphx commented 9 years ago

Sorry, not merging this. I disagree this is a good behavior. Not only that, it breaks backwards compatibility. If you'd like to hear my thoughts on instance_eval, I'd be happy to explain.

Arcovion commented 9 years ago

Sorry, I didn't think it was backwards incompatible. You could do:

if block.arity.zero?
  job.instance_eval(&block)
else
  yield job
end

To keep current behaviour, right?

I am not aware of what side effects the instance_eval causes in this case as I didn't look at the codebase, just added it as a quick modification. Please do explain your thoughts, thanks.

evanphx commented 9 years ago

That does fix the backwards compat issue. instance_eval changes the behavior of self and ivars so applying it to random blocks within a larger program is really confusing. I feel that using an explicit block var is more verbose but much easier to understand. The savings of using instance_eval here is very small and doesn't improve the overall usability of the library.

Arcovion commented 9 years ago

Yea I understand this is just a layer of magic, and I wasn't intending to break backwards compatability for users. To me this gem is a quick tool and not a permanent fixture in my code, it saves me from writing out iterations.times so I decided to go one step further with this, maybe too far =]

Thanks for your work on this gem, it's very handy!

Arcovion commented 9 years ago

Hi, I just found an interesting gem for this - iron-dsl Where yield job is, you can instead use:

  require 'iron/dsl'
  DslProxy.exec job, &block

This appears to make instance variables and self work correctly! Of course, it's a bit of a hack and I don't intend to use it myself, but I thought you might be interested to know about it - I didn't know it was possible. Cheers.