Permalink
Browse files

Make HashWithIndifferentAccess#select always return the hash.

Hash#select! returns nil if the hash didn't change and thus behaves differently
from select, so it's return value can't be used as result for the latter.
  • Loading branch information...
1 parent 01d4941 commit 20c065594f1434c93e6fe64cad062a0df1f42a8e @schuetzm schuetzm committed Jul 6, 2013
@@ -1,3 +1,8 @@
+* Make `HashWithIndifferentAccess#select` always return the hash, even when
+ `Hash#select!` returns `nil`, to allow further chaining.
+
+ *Marc Schütz*
+
* Remove deprecated `String#encoding_aware?` core extensions (`core_ext/string/encoding`).
*Arun Agrawal*
@@ -228,7 +228,7 @@ def deep_symbolize_keys; to_hash.deep_symbolize_keys! end
def to_options!; self end
def select(*args, &block)
- dup.select!(*args, &block)
+ dup.tap {|hash| hash.select!(*args, &block)}
end
# Convert to a regular hash with string keys.
@@ -487,6 +487,12 @@ def test_indifferent_select
assert_instance_of ActiveSupport::HashWithIndifferentAccess, hash
end
+ def test_indifferent_select_returns_a_hash_when_unchanged
+ hash = ActiveSupport::HashWithIndifferentAccess.new(@strings).select {|k,v| true}
+
+ assert_instance_of ActiveSupport::HashWithIndifferentAccess, hash
+ end
+
def test_indifferent_select_bang
indifferent_strings = ActiveSupport::HashWithIndifferentAccess.new(@strings)
indifferent_strings.select! {|k,v| v == 1}

1 comment on commit 20c0655

Just upgraded a project past this particular change. Always returning the hash itself causes HWIA to act differently from Hash (and most other Enumerable implementations) by not returning Enumerator when no block is given.

For instance, calling Hash.new.select will return #<Enumerator: {}:select>, which is how Hash.new.select.with_index does what it does. Until this patch, HWIA has functioned similarly.

This patch instead causes HashWithIndifferentAccess.new.select.with_index to raise NoMethodError: undefined method 'with_index' because #select (no args, no block) does not return <Enumerator {}:select> anymore.

This is a significant break in expectations for anyone who treats HWIA as a fully-featured implementation of Enumerable. Simply changing the implementation to the following seems to meet both of our use cases:

def select(*args, &block)
  copy = dup
  copy.select!(*args, &block) || copy
end

The method no longer returns nil when Hash#select would not, but Hash#select! would, and in the case where Hash#select should produce an Enumerator, HWIA will do so as well.

Edit: Opened issue #20095 to elaborate further. Will endeavor to have a test and pull request later in the week.

Please sign in to comment.