Skip to content
Browse files

Revert "Use flat_map { } instead of map {}.flatten"

This reverts commit abf8de8.
We should take a deeper look to those cases flat_map doesn't do deep
flattening.

irb(main):002:0> [[[1,3], [1,2]]].map{|i| i}.flatten
=> [1, 3, 1, 2]
irb(main):003:0> [[[1,3], [1,2]]].flat_map{|i| i}
=> [[1, 3], [1, 2]]
  • Loading branch information...
1 parent abf8de8 commit a0613ad8a9773c76a9b0a256f7099fde35823674 @spastorino spastorino committed
View
4 actionpack/lib/action_dispatch/routing/inspector.rb
@@ -102,9 +102,9 @@ def collect_engine_routes(route)
end
def formatted_routes_for_engines
- @engines.flat_map do |name, routes|
+ @engines.map do |name, routes|
["\nRoutes for #{name}:"] + formatted_routes(routes)
- end
+ end.flatten
end
def formatted_routes(routes)
View
4 activerecord/lib/active_record/associations/join_dependency.rb
@@ -35,12 +35,12 @@ def join_base
end
def columns
- join_parts.flat_map { |join_part|
+ join_parts.collect { |join_part|
table = join_part.aliased_table
join_part.column_names_with_alias.collect{ |column_name, aliased_name|
table[column_name].as Arel.sql(aliased_name)
}
- }
+ }.flatten
end
def instantiate(rows)
View
2 activerecord/lib/active_record/associations/preloader.rb
@@ -106,7 +106,7 @@ def preload(association)
def preload_hash(association)
association.each do |parent, child|
Preloader.new(records, parent, preload_scope).run
- Preloader.new(records.flat_map { |record| record.send(parent) }, child).run
+ Preloader.new(records.map { |record| record.send(parent) }.flatten, child).run
end
end
View
2 activerecord/lib/active_record/associations/preloader/association.rb
@@ -77,7 +77,7 @@ def associated_records_by_owner
# Some databases impose a limit on the number of ids in a list (in Oracle it's 1000)
# Make several smaller queries if necessary or make one query if the adapter supports it
sliced = owner_keys.each_slice(model.connection.in_clause_length || owner_keys.size)
- records = sliced.flat_map { |slice| records_for(slice).to_a }
+ records = sliced.map { |slice| records_for(slice).to_a }.flatten
end
# Each record may have multiple owners, and vice-versa
View
4 activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -448,7 +448,7 @@ def create_table(table_name, options = {}) #:nodoc:
end
def bulk_change_table(table_name, operations) #:nodoc:
- sqls = operations.flat_map do |command, args|
+ sqls = operations.map do |command, args|
table, arguments = args.shift, args
method = :"#{command}_sql"
@@ -457,7 +457,7 @@ def bulk_change_table(table_name, operations) #:nodoc:
else
raise "Unknown method called : #{method}(#{arguments.inspect})"
end
- end.join(", ")
+ end.flatten.join(", ")
execute("ALTER TABLE #{quote_table_name(table_name)} #{sqls}")
end
View
2 activerecord/lib/active_record/observer.rb
@@ -99,7 +99,7 @@ class Observer < ActiveModel::Observer
def observed_classes
klasses = super
- klasses + klasses.flat_map { |klass| klass.descendants }
+ klasses + klasses.map { |klass| klass.descendants }.flatten
end
def add_observer!(klass)
View
4 activerecord/lib/active_record/relation/query_methods.rb
@@ -782,7 +782,7 @@ def build_select(arel, selects)
def reverse_sql_order(order_query)
order_query = ["#{quoted_table_name}.#{quoted_primary_key} ASC"] if order_query.empty?
- order_query.flat_map do |o|
+ order_query.map do |o|
case o
when Arel::Nodes::Ordering
o.reverse
@@ -794,7 +794,7 @@ def reverse_sql_order(order_query)
else
o
end
- end
+ end.flatten
end
def array_of_strings?(o)
View
2 activerecord/test/cases/associations/callbacks_test.rb
@@ -138,7 +138,7 @@ def test_has_and_belongs_to_many_remove_callback_on_clear
activerecord.reload
assert activerecord.developers_with_callbacks.size == 2
end
- log_array = activerecord.developers_with_callbacks.flat_map {|d| ["before_removing#{d.id}","after_removing#{d.id}"]}.sort
+ log_array = activerecord.developers_with_callbacks.collect {|d| ["before_removing#{d.id}","after_removing#{d.id}"]}.flatten.sort
assert activerecord.developers_with_callbacks.clear
assert_equal log_array, activerecord.developers_log.sort
end
View
2 activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -597,7 +597,7 @@ def test_has_many_association_through_a_has_many_association_to_self
sarah = Person.create!(:first_name => 'Sarah', :primary_contact_id => people(:susan).id, :gender => 'F', :number1_fan_id => 1)
john = Person.create!(:first_name => 'John', :primary_contact_id => sarah.id, :gender => 'M', :number1_fan_id => 1)
assert_equal sarah.agents, [john]
- assert_equal people(:susan).agents.flat_map(&:agents), people(:susan).agents_of_agents
+ assert_equal people(:susan).agents.map(&:agents).flatten, people(:susan).agents_of_agents
end
def test_associate_existing_with_nonstandard_primary_key_on_belongs_to
View
4 activerecord/test/cases/autosave_association_test.rb
@@ -50,9 +50,9 @@ def assert_no_difference_when_adding_callbacks_twice_for(model, association_name
end
def callbacks_for_model(model)
- model.instance_variables.grep(/_callbacks$/).flat_map do |ivar|
+ model.instance_variables.grep(/_callbacks$/).map do |ivar|
model.instance_variable_get(ivar)
- end
+ end.flatten
end
end
View
4 activesupport/lib/active_support/multibyte/unicode.rb
@@ -219,9 +219,9 @@ def compose(codepoints)
# encoding is entirely CP1252 or ISO-8859-1.
def tidy_bytes(string, force = false)
if force
- return string.unpack("C*").flat_map do |b|
+ return string.unpack("C*").map do |b|
tidy_byte(b)
- end.compact.pack("C*").unpack("U*").pack("U*")
+ end.flatten.compact.pack("C*").unpack("U*").pack("U*")
end
bytes = string.unpack("C*")
View
2 guides/rails_guides/helpers.rb
@@ -17,7 +17,7 @@ def documents_by_section
end
def documents_flat
- documents_by_section.flat_map {|section| section['documents']}
+ documents_by_section.map {|section| section['documents']}.flatten
end
def finished_documents(documents)
View
6 guides/source/active_support_core_extensions.md
@@ -517,7 +517,7 @@ ActionController::TestCase.class_eval do
# now redefine process and delegate to original_process
def process(action, params=nil, session=nil, flash=nil, http_method='GET')
- params = Hash[*params.flat_map {|k, v| [k, v.to_s]}]
+ params = Hash[*params.map {|k, v| [k, v.to_s]}.flatten]
original_process(action, params, session, flash, http_method)
end
end
@@ -530,7 +530,7 @@ That technique has a risk, it could be the case that `:original_process` was tak
```ruby
ActionController::TestCase.class_eval do
def process_with_stringified_params(...)
- params = Hash[*params.flat_map {|k, v| [k, v.to_s]}]
+ params = Hash[*params.map {|k, v| [k, v.to_s]}.flatten]
process_without_stringified_params(action, params, session, flash, http_method)
end
alias_method :process_without_stringified_params, :process
@@ -543,7 +543,7 @@ The method `alias_method_chain` provides a shortcut for that pattern:
```ruby
ActionController::TestCase.class_eval do
def process_with_stringified_params(...)
- params = Hash[*params.flat_map {|k, v| [k, v.to_s]}]
+ params = Hash[*params.map {|k, v| [k, v.to_s]}.flatten]
process_without_stringified_params(action, params, session, flash, http_method)
end
alias_method_chain :process, :stringified_params
View
2 railties/lib/rails/generators/actions.rb
@@ -184,7 +184,7 @@ def initializer(filename, data=nil, &block)
# generate(:authenticated, "user session")
def generate(what, *args)
log :generate, what
- argument = args.flat_map {|arg| arg.to_s }.join(" ")
+ argument = args.map {|arg| arg.to_s }.flatten.join(" ")
in_root { run_ruby_script("script/rails generate #{what} #{argument}", :verbose => false) }
end
View
2 railties/lib/rails/paths.rb
@@ -103,7 +103,7 @@ def filter_by(constraint)
all_paths.each do |path|
if path.send(constraint)
paths = path.existent
- paths -= path.children.flat_map { |p| p.send(constraint) ? [] : p.existent }
+ paths -= path.children.map { |p| p.send(constraint) ? [] : p.existent }.flatten
all.concat(paths)
end
end
View
2 railties/lib/rails/source_annotation_extractor.rb
@@ -100,7 +100,7 @@ def extract_annotations_from(file, pattern)
# Prints the mapping from filenames to annotations in +results+ ordered by filename.
# The +options+ hash is passed to each annotation's +to_s+.
def display(results, options={})
- options[:indent] = results.flat_map { |f, a| a.map(&:line) }.max.to_s.size
+ options[:indent] = results.map { |f, a| a.map(&:line) }.flatten.max.to_s.size
results.keys.sort.each do |file|
puts "#{file}:"
results[file].each do |note|
View
4 railties/lib/rails/test_unit/testing.rake
@@ -6,7 +6,7 @@ TEST_CHANGES_SINCE = Time.now - 600
# Look up tests for recently modified sources.
def recent_tests(source_pattern, test_path, touched_since = 10.minutes.ago)
- FileList[source_pattern].flat_map do |path|
+ FileList[source_pattern].map do |path|
if File.mtime(path) > touched_since
tests = []
source_dir = File.dirname(path).split("/")
@@ -26,7 +26,7 @@ def recent_tests(source_pattern, test_path, touched_since = 10.minutes.ago)
return tests
end
- end.compact
+ end.flatten.compact
end

2 comments on commit a0613ad

@mking

FWIW, I believe flat_map is identical to map {|x| Array(x)}.inject(&:concat) ("map, treat the result as an Array, and concat").

irb(main):026:0> [1,[2,3],[[4, [5]], 6]].flat_map {|x| x}
=> [1, 2, 3, [4, [5]], 6]
irb(main):027:0> [1,[2,3],[[4, [5]], 6]].map {|x| Array(x)}.inject(&:concat)
=> [1, 2, 3, [4, [5]], 6]
@spastorino
Ruby on Rails member

@mking exactly, that's why I reverted the commit.
Anyway, flat_map is fine in most of the cases, though. We would need to investigate case by case properly.

Please sign in to comment.
Something went wrong with that request. Please try again.