public
Description: Ruby on Rails
Homepage: http://rubyonrails.org
Clone URL: git://github.com/rails/rails.git
Hash#slice supports an array of keys [#613 state:resolved]

Signed-off-by: Joshua Peek <josh@joshpeek.com>
queso (author)
Wed Jul 16 17:31:37 -0700 2008
josh (committer)
Wed Jul 16 17:31:37 -0700 2008
commit  396f9df8916b71f83aad8d56559cf55fc8501679
tree    05361067dd4c91f048493939fa8dbda442ec52e4
parent  7ae2105d57d3c08bebde44bd72093dd43c48d613
...
12
13
14
 
15
16
17
...
12
13
14
15
16
17
18
0
@@ -12,6 +12,7 @@ module ActiveSupport #:nodoc:
0
       module Slice
0
         # Returns a new hash with only the given keys.
0
         def slice(*keys)
0
+          keys.flatten!
0
           keys = keys.map! { |key| convert_key(key) } if respond_to?(:convert_key)
0
           hash = {}
0
           keys.each { |k| hash[k] = self[k] if has_key?(k) }
...
282
283
284
 
 
 
 
 
 
 
 
 
 
 
 
 
 
285
286
287
...
469
470
471
472
 
473
474
475
476
477
 
478
479
480
...
552
553
554
555
 
556
557
558
...
650
651
652
653
 
654
655
656
657
658
659
 
660
661
662
...
665
666
667
668
 
669
670
671
...
699
700
701
702
703
 
 
704
705
706
707
708
 
709
710
711
 
 
712
713
714
 
715
716
717
718
 
719
720
721
722
 
 
723
724
725
726
727
 
728
729
730
 
 
731
732
733
734
735
736
 
737
738
739
...
744
745
746
747
 
748
749
750
...
755
756
757
758
 
759
760
761
...
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
...
483
484
485
 
486
487
488
489
490
 
491
492
493
494
...
566
567
568
 
569
570
571
572
...
664
665
666
 
667
668
669
670
671
672
 
673
674
675
676
...
679
680
681
 
682
683
684
685
...
713
714
715
 
 
716
717
718
719
720
721
 
722
723
 
 
724
725
726
727
 
728
729
730
731
 
732
733
734
 
 
735
736
737
738
739
740
 
741
742
 
 
743
744
745
746
747
748
749
 
750
751
752
753
...
758
759
760
 
761
762
763
764
...
769
770
771
 
772
773
774
775
0
@@ -282,6 +282,20 @@ class HashExtTest < Test::Unit::TestCase
0
     assert_equal expected, original
0
   end
0
 
0
+  # This is needed for something like hash.slice!(hash.keys.sort_by {rand} [0..4])
0
+  def test_slice_with_array_keys
0
+    original = { :a => 'x', :b => 'y', :c => 10 }
0
+    expected = { :a => 'x', :b => 'y' }
0
+
0
+    # Should return a new hash with only the given keys, when given an array of keys.
0
+    assert_equal expected, original.slice([:a, :b])
0
+    assert_not_equal expected, original
0
+
0
+    # Should replace the hash with only the given keys, when given an array of keys.
0
+    assert_equal expected, original.slice!([:a, :b])
0
+    assert_equal expected, original
0
+  end
0
+
0
   def test_indifferent_slice
0
     original = { :a => 'x', :b => 'y', :c => 10 }.with_indifferent_access
0
     expected = { :a => 'x', :b => 'y' }.with_indifferent_access
0
@@ -469,12 +483,12 @@ class HashToXmlTest < Test::Unit::TestCase
0
     EOT
0
 
0
     expected_topic_hash = {
0
-      :title      => nil, 
0
+      :title      => nil,
0
       :id         => nil,
0
       :approved   => nil,
0
       :written_on => nil,
0
       :viewed_at  => nil,
0
-      :content    => nil, 
0
+      :content    => nil,
0
       :parent_id  => nil
0
     }.stringify_keys
0
 
0
@@ -552,7 +566,7 @@ class HashToXmlTest < Test::Unit::TestCase
0
 
0
     assert_equal expected_topic_hash, Hash.from_xml(topic_xml)["rsp"]["photos"]["photo"]
0
   end
0
-  
0
+
0
   def test_empty_array_from_xml
0
     blog_xml = <<-XML
0
       <blog>
0
@@ -650,13 +664,13 @@ class HashToXmlTest < Test::Unit::TestCase
0
 
0
     assert_equal expected_bacon_hash, Hash.from_xml(bacon_xml)["bacon"]
0
   end
0
-  
0
+
0
   def test_type_trickles_through_when_unknown
0
     product_xml = <<-EOT
0
     <product>
0
       <weight type="double">0.5</weight>
0
       <image type="ProductImage"><filename>image.gif</filename></image>
0
-      
0
+
0
     </product>
0
     EOT
0
 
0
@@ -665,7 +679,7 @@ class HashToXmlTest < Test::Unit::TestCase
0
       :image => {'type' => 'ProductImage', 'filename' => 'image.gif' },
0
     }.stringify_keys
0
 
0
-    assert_equal expected_product_hash, Hash.from_xml(product_xml)["product"]    
0
+    assert_equal expected_product_hash, Hash.from_xml(product_xml)["product"]
0
   end
0
 
0
   def test_should_use_default_value_for_unknown_key
0
@@ -699,41 +713,41 @@ class HashToXmlTest < Test::Unit::TestCase
0
       assert_equal expected, hash.to_xml(@xml_options)
0
     end
0
   end
0
-  
0
-  def test_empty_string_works_for_typecast_xml_value    
0
+
0
+  def test_empty_string_works_for_typecast_xml_value
0
     assert_nothing_raised do
0
       Hash.send!(:typecast_xml_value, "")
0
     end
0
   end
0
-  
0
+
0
   def test_escaping_to_xml
0
-    hash = { 
0
-      :bare_string        => 'First & Last Name', 
0
+    hash = {
0
+      :bare_string        => 'First & Last Name',
0
       :pre_escaped_string => 'First &amp; Last Name'
0
     }.stringify_keys
0
-    
0
+
0
     expected_xml = '<person><bare-string>First &amp; Last Name</bare-string><pre-escaped-string>First &amp;amp; Last Name</pre-escaped-string></person>'
0
     assert_equal expected_xml, hash.to_xml(@xml_options)
0
   end
0
-  
0
+
0
   def test_unescaping_from_xml
0
     xml_string = '<person><bare-string>First &amp; Last Name</bare-string><pre-escaped-string>First &amp;amp; Last Name</pre-escaped-string></person>'
0
-    expected_hash = { 
0
-      :bare_string        => 'First & Last Name', 
0
+    expected_hash = {
0
+      :bare_string        => 'First & Last Name',
0
       :pre_escaped_string => 'First &amp; Last Name'
0
     }.stringify_keys
0
     assert_equal expected_hash, Hash.from_xml(xml_string)['person']
0
   end
0
-  
0
+
0
   def test_roundtrip_to_xml_from_xml
0
-    hash = { 
0
-      :bare_string        => 'First & Last Name', 
0
+    hash = {
0
+      :bare_string        => 'First & Last Name',
0
       :pre_escaped_string => 'First &amp; Last Name'
0
     }.stringify_keys
0
 
0
     assert_equal hash, Hash.from_xml(hash.to_xml(@xml_options))['person']
0
   end
0
-  
0
+
0
   def test_datetime_xml_type_with_utc_time
0
     alert_xml = <<-XML
0
       <alert>
0
@@ -744,7 +758,7 @@ class HashToXmlTest < Test::Unit::TestCase
0
     assert alert_at.utc?
0
     assert_equal Time.utc(2008, 2, 10, 15, 30, 45), alert_at
0
   end
0
-  
0
+
0
   def test_datetime_xml_type_with_non_utc_time
0
     alert_xml = <<-XML
0
       <alert>
0
@@ -755,7 +769,7 @@ class HashToXmlTest < Test::Unit::TestCase
0
     assert alert_at.utc?
0
     assert_equal Time.utc(2008, 2, 10, 15, 30, 45), alert_at
0
   end
0
-  
0
+
0
   def test_datetime_xml_type_with_far_future_date
0
     alert_xml = <<-XML
0
       <alert>

Comments

skojin Wed Jul 16 22:47:44 -0700 2008

That will broke behavior when I use array as key original = {[5, 6] => ‘x’, [2, 3] => ‘y’, [1, 6] => 10 }

I think people can always use original.slice(*[:a, :b])

josh Wed Jul 16 23:11:28 -0700 2008

Could do Array(...) instead of flatten. I think that would work for both cases. Submit a patch to Lighthouse and I’ll have a look.

matthewrudy Thu Jul 17 05:13:32 -0700 2008

Yeah, I think this is crap.

if you have an array you should just splat it.

if you legitimately give it an array as a key it should be allowed

tomafro Thu Jul 17 05:21:28 -0700 2008

What sergey and matthew said. The implicit assumption that arrays aren’t used as keys within hashes is incorrect. The original implementation of slice was correct.

matthewrudy Thu Jul 17 06:16:25 -0700 2008

I’ve added a test to assert the original behaviour (added to the original ticket) http://rails.lighthouseapp.com/projects/8994/tickets/613-slice-and-slice-don-t-accept-an-array-of-keys#ticket-613-6