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

string/concatenation.rb tests are misleading #64

Open
Aqualon opened this issue Aug 10, 2015 · 3 comments
Open

string/concatenation.rb tests are misleading #64

Aqualon opened this issue Aug 10, 2015 · 3 comments

Comments

@Aqualon
Copy link

Aqualon commented Aug 10, 2015

Hi, the tests in string/concatenation.rb are quite misleading.

The fast method consists of this

def fast
  'foo' 'bar'
end

That's not concatenating during calling of fast but on parsing the code. If you write another method just returning foobar, it is as fast as this method.

So I think this is not fair comparison and what you usually want is to concatenate two variables during runtime.

For this use case concat and << are calling the same code, so they have the same performance and both are fine if you want to change the string on the left and not just get two strings concatenated. If you want a new string you can use +.

Some better test could be to compare + and String interpolation

Benchmark.ips do |x|
  foo = 'foo'
  bar = 'bar'

  x.report('String#+') do
    foo + bar
  end

  x.report('String interpolation') do
    "#{foo}#{bar}"
    end

  x.compare!
end

This still has the difference that interpolation can handle nil values, while + cannot.

@mblumtritt
Copy link

👍

@JuanitoFatas
Copy link
Contributor

Hi @Aqualon, I took "foo" "bar" example from https://github.com/rspec/rspec-core/blob/34c2c750c48cf880b7194e3e295236afd92c0ca8/lib/rspec/core/configuration.rb#L147-L151, also see http://thepugautomatic.com/2015/01/backslashy-ruby/#comment-1780720281.

Maybe this just a bad naming, do you have better name for current example? Or add explanatory text before this benchmark?

Thanks for raising this.

@hannesfostie
Copy link

@JuanitoFatas I was just running through a project trying to find optimizations, and found something that fit the string concatenation example. I blindly took your results and applied them, basically changing this:

    headers = {'Cache-Control' => 'foobar'}
    headers['Cache-Control'] += ', no-transform'

to this:

    headers = {'Cache-Control' => 'foobar'}
    headers['Cache-Control'] = "#{headers['Cache-Control']}, no-transform"

Luckily I am benchmarking everything, so I noticed there's no difference. The reason is fairly obvious of course, as I'm changing the object rather than creating a new object like you are in your examples.

I think this deserves some explanation in the example?

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

4 participants