Skip to content

Commit

Permalink
Faster Hash#slice that doesn't use Enumerable#include?.
Browse files Browse the repository at this point in the history
Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information
chuyeow authored and jeremy committed Jun 2, 2008
1 parent 714d42d commit 952ec79
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion activesupport/lib/active_support/core_ext/hash/slice.rb
Expand Up @@ -15,7 +15,9 @@ module Slice
# Returns a new hash with only the given keys.
def slice(*keys)
allowed = Set.new(respond_to?(:convert_key) ? keys.map { |key| convert_key(key) } : keys)
reject { |key,| !allowed.include?(key) }
hash = {}
allowed.each { |k| hash[k] = self[k] }
hash
end

# Replaces the hash with only the given keys.
Expand Down

6 comments on commit 952ec79

@denis
Copy link

@denis denis commented on 952ec79 Jun 2, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has broke form helpers!
Now they generate something like name=“list[][name]” instead of name=“list[name]”.
All forms in my app are broken.

@denis
Copy link

@denis denis commented on 952ec79 Jun 2, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

denis@denis-laptop:~/dev/ruby/rails/actionpack$ rake
1966 tests, 9460 assertions, 16 failures, 0 errors

@methodmissing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see http://github.com/methodmissing/rails/commit/fee25cfcb6c8a29ff1906d54ba849349f38ebedd for the fix

@djd
Copy link

@djd djd commented on 952ec79 Jun 2, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the returning(…) syntax?

returning {} do |allowed|
allowed.each { |k| hash[k] = self[k] }
end

@josh
Copy link
Contributor

@josh josh commented on 952ec79 Jun 2, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We get shit for that (performance). Looks much nicer but I guess you don’t need it for really low level stuff like this.

@djd
Copy link

@djd djd commented on 952ec79 Jun 2, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, thanks for the clarification.

Please sign in to comment.