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

Replace map + compact with filter_map #42053

Merged

Conversation

MatheusRich
Copy link
Contributor

Summary

Since Rails 7 requires Ruby > 2.7, I changed some recurrences of map + compact with filter_map. I took the liberty to change some occurrences in tests too.

Other Information

Here's an article about filter_map: https://blog.saeloun.com/2019/05/25/ruby-2-7-enumerable-filter-map.html

Benchmark

N = 1_00_000
enum = 1.upto(1_000)
Benchmark.bmbm do |x|
  x.report("select + map") {N.times {enum.select { |i| i.even? }.map{|i| i +1}}}
  x.report("map + compact") {N.times {enum.map { |i| i + 1 if i.even? }.compact}}
  x.report("filter_map") {N.times {enum.filter_map { |i| i + 1 if i.even? }}}
end
#
# Rehearsal -------------------------------------------------
# select + map    8.569651   0.051319   8.620970 (  8.632449)
# map + compact   7.392666   0.133964   7.526630 (  7.538013)
# filter_map      6.923772   0.022314   6.946086 (  6.956135)
# --------------------------------------- total: 23.093686sec
# 
#                     user     system      total        real
# select + map    8.550637   0.033190   8.583827 (  8.597627)
# map + compact   7.263667   0.131180   7.394847 (  7.405570)
# filter_map      6.761388   0.018223   6.779611 (  6.790559)

@MatheusRich MatheusRich force-pushed the replace-map-compact-with-filter-map branch from 795c4b0 to c3d7794 Compare April 23, 2021 01:09
@rafaelfranca rafaelfranca merged commit 7d88c86 into rails:main Apr 23, 2021
@MatheusRich MatheusRich deleted the replace-map-compact-with-filter-map branch April 23, 2021 02:18
kamipo added a commit that referenced this pull request Apr 23, 2021
@adityapandit17
Copy link
Contributor

adityapandit17 commented Apr 23, 2021

@rafaelfranca this is really wrong with this merge.
When I raise a pull request with exact same changes you reject. But now you have merged the same chages
@dhh @eileencodes @tenderlove @georgeclaghorn this is really partial.
link to the pull request

this is the link to the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants