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

Add `Hash#map_values` to ActiveSupport to simplify a common pattern #15819

Merged
merged 1 commit into from Jun 29, 2014
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -1,3 +1,8 @@
* Add `Hash#transform_values` to simplify a common pattern where the values of a
hash must change, but the keys are left the same.

*Sean Griffin*

* Always instrument `ActiveSupport::Cache`.

Since `ActiveSupport::Notifications` only instrument items when there
@@ -6,3 +6,4 @@
require 'active_support/core_ext/hash/keys'
require 'active_support/core_ext/hash/reverse_merge'
require 'active_support/core_ext/hash/slice'
require 'active_support/core_ext/hash/transform_values'

This comment has been minimized.

Copy link
@egilburg

egilburg Jun 20, 2014

Contributor

transform_keys and friends live in /core_ext/hash/keys.rb. To be consistent, this file could be called /core_ext/hash/values.rb.

@@ -0,0 +1,21 @@
class Hash
# Returns a new hash with the results of running +block+ once for every value.
# The keys are unchanged.
#
# { a: 1, b: 2, c: 3 }.transform_values { |x| x * 2 }
# # => { a: 2, b: 4, c: 6 }
def transform_values(&block)

This comment has been minimized.

Copy link
@bkuhlmann

bkuhlmann Jul 12, 2014

Contributor

In case it helps, maybe refactor the code as follows?:

def transform_values(&block)
  each_with_object(self.class.new) do |(key, value), result|
    result[key] = yield value
  end
end

This would eliminate the need for an instance variable.

This comment has been minimized.

Copy link
@matthewd

matthewd Jul 12, 2014

Member

It's a local variable, not an instance variable. And that spelling would severely curtail the advantage of this method: it currently avoids doing an allocation per hash entry.

result = self.class.new
each do |key, value|
result[key] = yield(value)
end
result
end

# Destructive +transform_values+
def transform_values!
each do |key, value|
self[key] = yield(value)
end
end
end
@@ -0,0 +1,49 @@
require 'abstract_unit'
require 'active_support/core_ext/hash/indifferent_access'
require 'active_support/core_ext/hash/transform_values'

class TransformValuesTest < ActiveSupport::TestCase
test "transform_values returns a new hash with the values computed from the block" do
original = { a: 'a', b: 'b' }
mapped = original.transform_values { |v| v + '!' }

assert_equal({ a: 'a', b: 'b' }, original)
assert_equal({ a: 'a!', b: 'b!' }, mapped)
end

test "transform_values! modifies the values of the original" do
original = { a: 'a', b: 'b' }
mapped = original.transform_values! { |v| v + '!' }

assert_equal({ a: 'a!', b: 'b!' }, original)
assert_same original, mapped
end

test "indifferent access is still indifferent after mapping values" do
original = { a: 'a', b: 'b' }.with_indifferent_access
mapped = original.transform_values { |v| v + '!' }

assert_equal 'a!', mapped[:a]
assert_equal 'a!', mapped['a']
end

# This is to be consistent with the behavior of Ruby's built in methods
# (e.g. #select, #reject) as of 2.2
test "default values do not persist during mapping" do
original = Hash.new('foo')
original[:a] = 'a'
mapped = original.transform_values { |v| v + '!' }

assert_equal 'a!', mapped[:a]
assert_nil mapped[:b]
end

test "default procs do not persist after mapping" do
original = Hash.new { 'foo' }
original[:a] = 'a'
mapped = original.transform_values { |v| v + '!' }

assert_equal 'a!', mapped[:a]
assert_nil mapped[:b]
end
end
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.