Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add new Performance/MapMethodChain cop #364

Merged
merged 1 commit into from
Aug 13, 2023

Conversation

koic
Copy link
Member

@koic koic commented Aug 11, 2023

This PR adds new Performance/MapMethodChain cop that checks if the map method is used in a chain.

# bad
array.map(&:foo).map(&:bar)

# good
array.map { |item| item.foo.bar }

Safety

This cop is unsafe because false positives occur if the number of times the first method is executed affects the return value of subsequent methods.

class X
  def initialize
    @@num = 0
  end

  def foo
    @@num += 1
    self
  end

  def bar
    @@num * 2
  end
end

[X.new, X.new].map(&:foo).map(&:bar) # => [4, 4]
[X.new, X.new].map { |x| x.foo.bar } # => [2, 4]

Benchmark

$ cat map_chain.rb
#!/usr/local/bin/ruby

require 'benchmark/ips'

Benchmark.ips do |x|
ary = %i[hi bye]

x.report('map.map') { ary.map(&:to_s).map(&:length) }
x.report('map')     { ary.map { |item| item.to_s.length } }

x.compare!
end
$ ruby -v
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-darwin19]

$ ruby map_chain.rb
Warming up --------------------------------------
map.map   215.877k i/100ms
map   319.746k i/100ms
Calculating -------------------------------------
map.map      2.116M (± 2.1%) i/s -     10.578M in   5.002379s
map      3.227M (± 2.8%) i/s -     16.307M in   5.058091s

Comparison:
map:  3226860.1 i/s
map.map:  2115551.0 i/s - 1.53x  slower

Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

This PR adds new `Performance/MapMethodChain` cop that checks if the map method is used in a chain.

```ruby
# bad
array.map(&:foo).map(&:bar)

# good
array.map { |item| item.foo.bar }
```

## Safety

This cop is unsafe because false positives occur if the number of times the first method is executed
affects the return value of subsequent methods.

```ruby
class X
  def initialize
    @@num = 0
  end

  def foo
    @@num += 1
    self
  end

  def bar
    @@num * 2
  end
end

[X.new, X.new].map(&:foo).map(&:bar) # => [4, 4]
[X.new, X.new].map { |x| x.foo.bar } # => [2, 4]
```
## Benchmark

```console
$ cat map_chain.rb
#!/usr/local/bin/ruby

require 'benchmark/ips'

Benchmark.ips do |x|
ary = %i[hi bye]

x.report('map.map') { ary.map(&:to_s).map(&:length) }
x.report('map')     { ary.map { |item| item.to_s.length } }

x.compare!
end
```

```console
$ ruby -v
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-darwin19]

$ ruby map_chain.rb
Warming up --------------------------------------
map.map   215.877k i/100ms
map   319.746k i/100ms
Calculating -------------------------------------
map.map      2.116M (± 2.1%) i/s -     10.578M in   5.002379s
map      3.227M (± 2.8%) i/s -     16.307M in   5.058091s

Comparison:
map:  3226860.1 i/s
map.map:  2115551.0 i/s - 1.53x  slower
```
@koic koic merged commit 6b4d243 into rubocop:master Aug 13, 2023
4 checks passed
@koic koic deleted the add_new_performance_map_method_chain_cop branch August 13, 2023 08:28
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant