Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Remove some Symbol#to_proc usage in runtime code. [#484 state:resolved]

  • Loading branch information...
commit ce4a1bb8538bd7cc5ee3cbf1156dc587482a7839 1 parent 11252e3
@chuyeow chuyeow authored jeremy committed
View
12 actionpack/lib/action_controller/base.rb
@@ -412,7 +412,7 @@ def controller_path
# More methods can be hidden using <tt>hide_actions</tt>.
def hidden_actions
unless read_inheritable_attribute(:hidden_actions)
- write_inheritable_attribute(:hidden_actions, ActionController::Base.public_instance_methods.map(&:to_s))
+ write_inheritable_attribute(:hidden_actions, ActionController::Base.public_instance_methods.map { |m| m.to_s })
end
read_inheritable_attribute(:hidden_actions)
@@ -420,12 +420,12 @@ def hidden_actions
# Hide each of the given methods from being callable as actions.
def hide_action(*names)
- write_inheritable_attribute(:hidden_actions, hidden_actions | names.map(&:to_s))
+ write_inheritable_attribute(:hidden_actions, hidden_actions | names.map { |name| name.to_s })
end
- ## View load paths determine the bases from which template references can be made. So a call to
- ## render("test/template") will be looked up in the view load paths array and the closest match will be
- ## returned.
+ # View load paths determine the bases from which template references can be made. So a call to
+ # render("test/template") will be looked up in the view load paths array and the closest match will be
+ # returned.
def view_paths
@view_paths || superclass.view_paths
end
@@ -1201,7 +1201,7 @@ def action_methods
end
def self.action_methods
- @action_methods ||= Set.new(public_instance_methods.map(&:to_s)) - hidden_actions
+ @action_methods ||= Set.new(public_instance_methods.map { |m| m.to_s }) - hidden_actions
end
def add_variables_to_assigns
View
16 actionpack/lib/action_controller/filters.rb
@@ -127,9 +127,9 @@ def should_not_skip?(controller)
def included_in_action?(controller, options)
if options[:only]
- Array(options[:only]).map(&:to_s).include?(controller.action_name)
+ Array(options[:only]).map { |o| o.to_s }.include?(controller.action_name)
elsif options[:except]
- !Array(options[:except]).map(&:to_s).include?(controller.action_name)
+ !Array(options[:except]).map { |o| o.to_s }.include?(controller.action_name)
else
true
end
@@ -544,13 +544,21 @@ def filter_chain
# Returns all the before filters for this class and all its ancestors.
# This method returns the actual filter that was assigned in the controller to maintain existing functionality.
def before_filters #:nodoc:
- filter_chain.select(&:before?).map(&:method)
+ filters = []
@tsaleh
tsaleh added a note

The speed increase here can’t possibly be worth the lack of readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ filter_chain.each do |filter|
+ filters << filter.method if filter.before?
+ end
+ filters
end
# Returns all the after filters for this class and all its ancestors.
# This method returns the actual filter that was assigned in the controller to maintain existing functionality.
def after_filters #:nodoc:
- filter_chain.select(&:after?).map(&:method)
+ filters = []
+ filter_chain.each do |filter|
+ filters << filter.method if filter.after?
+ end
+ filters
end
end
View
4 activerecord/lib/active_record/associations.rb
@@ -1145,7 +1145,7 @@ def collection_reader_method(reflection, association_proxy_class)
end
define_method("#{reflection.name.to_s.singularize}_ids") do
- send(reflection.name).map(&:id)
+ send(reflection.name).map { |record| record.id }
end
end
@@ -1490,7 +1490,7 @@ def construct_finder_sql_for_association_limiting(options, join_dependency)
sql << " FROM #{connection.quote_table_name table_name} "
if is_distinct
- sql << distinct_join_associations.collect(&:association_join).join
+ sql << distinct_join_associations.collect { |assoc| assoc.association_join }.join
add_joins!(sql, options, scope)
end
View
2  activerecord/lib/active_record/associations/association_collection.rb
@@ -14,7 +14,7 @@ def find(*args)
# If using a custom finder_sql, scan the entire collection.
if @reflection.options[:finder_sql]
expects_array = args.first.kind_of?(Array)
- ids = args.flatten.compact.uniq.map(&:to_i)
+ ids = args.flatten.compact.uniq.map { |arg| arg.to_i }
if ids.size == 1
id = ids.first
View
4 activerecord/lib/active_record/associations/has_many_association.rb
@@ -61,9 +61,9 @@ def insert_record(record)
def delete_records(records)
case @reflection.options[:dependent]
when :destroy
- records.each(&:destroy)
+ records.each { |r| r.destroy }
when :delete_all
- @reflection.klass.delete(records.map(&:id))
+ @reflection.klass.delete(records.map { |record| record.id })
else
ids = quoted_record_ids(records)
@reflection.klass.update_all(
View
2  activesupport/lib/active_support/core_ext/module/introspection.rb
@@ -70,6 +70,6 @@ def local_constants #:nodoc:
# Returns the names of the constants defined locally rather than the
# constants themselves. See <tt>local_constants</tt>.
def local_constant_names
- local_constants.map(&:to_s)
+ local_constants.map { |c| c.to_s }
end
end
View
2  activesupport/lib/active_support/core_ext/object/instance_variables.rb
@@ -35,7 +35,7 @@ def instance_values #:nodoc:
# C.new(0, 1).instance_variable_names # => ["@y", "@x"]
if RUBY_VERSION >= '1.9'
def instance_variable_names
- instance_variables.map(&:to_s)
+ instance_variables.map { |var| var.to_s }
end
else
alias_method :instance_variable_names, :instance_variables
View
4 activesupport/lib/active_support/dependencies.rb
@@ -387,7 +387,7 @@ def new_constants_in(*descs)
ensure
# Remove the stack frames that we added.
if defined?(watch_frames) && ! watch_frames.blank?
- frame_ids = watch_frames.collect(&:object_id)
+ frame_ids = watch_frames.collect { |frame| frame.object_id }
constant_watch_stack.delete_if do |watch_frame|
frame_ids.include? watch_frame.object_id
end
@@ -437,7 +437,7 @@ def remove_constant(const) #:nodoc:
protected
def log_call(*args)
if defined?(RAILS_DEFAULT_LOGGER) && RAILS_DEFAULT_LOGGER && log_activity
- arg_str = args.collect(&:inspect) * ', '
+ arg_str = args.collect { |arg| arg.inspect } * ', '
/in `([a-z_\?\!]+)'/ =~ caller(1).first
selector = $1 || '<unknown>'
log "called #{selector}(#{arg_str})"

6 comments on commit ce4a1bb

@tsaleh

The speed increase here can’t possibly be worth the lack of readability.

@jherdman

wouldn’t inject be more succinct? e.g.


  filter_chain.inject(Array.new) do |filters, filter_item|
    filters << filter_item if filter_item.before?
    filters
  end
@Aupajo

Believe it or not, those calls are sped up significantly :) See: http://blog.hasmanythrough.com/2006/3/7/symbol-to-proc-shorthand

@michaelklishin

Go for it.

@masterkain
                           user     system      total        real
Using Symbol#to_proc   2.860000   0.760000   3.620000 (  3.702233)
Standard call          2.740000   1.050000   3.790000 (  3.867797)
@evan

None of these appear to be in inner loops. What am I my missing?

@NZKoz
Owner

The best way to raise questions about this changeset is to either email jeremy and chuyeow and ask, or to ask on the core list.

Me, I have no idea either ;)

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