Skip to content

Commit

Permalink
Freeze strings in array constants and begin moving
Browse files Browse the repository at this point in the history
from using global constants to frozen strings (cc @avit).

It's not pretty, but it's faster and better for understanding the code
without having to look up what the special constants represent.

From the discussion in #530:

"Things are evolving, but with Ruby 2.2 here is my current
understanding:

- I believe you are correct that the strings inside the array could
benefit from freezing, in hot spots.

- Freezing a string may now be faster than looking up a frozen string
constant.

So, it might make sense now in Ransack master and upcoming releases, to
optimize for Ruby 2.2+ and stop using frozen constants, and replace
them with frozen strings or symbols."

Closes #530.
  • Loading branch information
jonatack committed Aug 29, 2015
1 parent 5686c5c commit 381a83c
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 67 deletions.
10 changes: 5 additions & 5 deletions lib/ransack/adapters/active_record/3.0/compat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,16 @@ def visit_Arel_Nodes_NamedFunction o
"#{
o.name
}(#{
o.distinct ? Ransack::Constants::DISTINCT : Ransack::Constants::EMPTY
o.distinct ? Ransack::Constants::DISTINCT : ''.freeze
}#{
o.expressions.map { |x| visit x }.join(Ransack::Constants::COMMA_SPACE)
o.expressions.map { |x| visit x }.join(', '.freeze)
})#{
o.alias ? " AS #{visit o.alias}" : Ransack::Constants::EMPTY
o.alias ? " AS #{visit o.alias}" : ''.freeze
}"
end

def visit_Arel_Nodes_And o
o.children.map { |x| visit x }.join(Ransack::Constants::SPACED_AND)
o.children.map { |x| visit x }.join(' AND '.freeze)
end

def visit_Arel_Nodes_Not o
Expand All @@ -164,7 +164,7 @@ def visit_Arel_Nodes_Values o
quote(value, attr && column_for(attr))
end
}
.join(Ransack::Constants::COMMA_SPACE)
.join(', '.freeze)
})"
end
end
Expand Down
33 changes: 11 additions & 22 deletions lib/ransack/constants.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
module Ransack
module Constants
ASC = 'asc'.freeze
DESC = 'desc'.freeze
ASC_DESC = [ASC, DESC].freeze

ASC_ARROW = '▲'.freeze
DESC_ARROW = '▼'.freeze

OR = 'or'.freeze
AND = 'and'.freeze
SPACED_AND = ' AND '.freeze

SORT = 'sort'.freeze
SORT_LINK = 'sort_link'.freeze
SORT_DIRECTION = 'sort_direction'.freeze

CAP_SEARCH = 'Search'.freeze
SEARCH = 'search'.freeze
Expand All @@ -23,17 +14,12 @@ module Constants
ATTRIBUTES = 'attributes'.freeze
COMBINATOR = 'combinator'.freeze

SPACE = ' '.freeze
COMMA_SPACE = ', '.freeze
COLON_SPACE = ': '.freeze
TWO_COLONS = '::'.freeze
UNDERSCORE = '_'.freeze
LEFT_PARENTHESIS = '('.freeze
Q = 'q'.freeze
I = 'i'.freeze
NON_BREAKING_SPACE = ' '.freeze
DOT_ASTERIX = '.*'.freeze
EMPTY = ''.freeze

STRING_JOIN = 'string_join'.freeze
ASSOCIATION_JOIN = 'association_join'.freeze
Expand All @@ -44,14 +30,17 @@ module Constants
FALSE_VALUES = [false, 0, '0', 'f', 'F', 'false', 'FALSE'].to_set
BOOLEAN_VALUES = (TRUE_VALUES + FALSE_VALUES).freeze

S_SORTS = %w(s sorts).freeze
AND_OR = %w(and or).freeze
IN_NOT_IN = %w(in not_in).freeze
SUFFIXES = %w(_any _all).freeze
AREL_PREDICATES = %w(
eq not_eq matches does_not_match lt lteq gt gteq in not_in
).freeze
A_S_I = %w(a s i).freeze
AND_OR = ['and'.freeze, 'or'.freeze].freeze
IN_NOT_IN = ['in'.freeze, 'not_in'.freeze].freeze
SUFFIXES = ['_any'.freeze, '_all'.freeze].freeze
AREL_PREDICATES = [
'eq'.freeze, 'not_eq'.freeze,
'matches'.freeze, 'does_not_match'.freeze,
'lt'.freeze, 'lteq'.freeze,
'gt'.freeze, 'gteq'.freeze,
'in'.freeze, 'not_in'.freeze
].freeze
A_S_I = ['a'.freeze, 's'.freeze, 'i'.freeze].freeze

EQ = 'eq'.freeze
NOT_EQ = 'not_eq'.freeze
Expand Down
26 changes: 14 additions & 12 deletions lib/ransack/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ def for_object(object, options = {})
end

def for(object, options = {})
context = Class === object ?
for_class(object, options) :
for_object(object, options)
context =
if Class === object
for_class(object, options)
else
for_object(object, options)
end
context or raise ArgumentError,
"Don't know what context to use for #{object}"
end
Expand Down Expand Up @@ -59,7 +62,7 @@ def bind(object, str)
end

def traverse(str, base = @base)
str ||= Constants::EMPTY
str ||= ''.freeze

if (segments = str.split(/_/)).size > 0
remainder = []
Expand All @@ -68,13 +71,12 @@ def traverse(str, base = @base)
# Strip the _of_Model_type text from the association name, but hold
# onto it in klass, for use as the next base
assoc, klass = unpolymorphize_association(
segments.join(Constants::UNDERSCORE)
segments.join('_'.freeze)
)
if found_assoc = get_association(assoc, base)
base = traverse(
remainder.join(
Constants::UNDERSCORE), klass || found_assoc.klass
)
remainder.join('_'.freeze), klass || found_assoc.klass
)
end

remainder.unshift segments.pop
Expand All @@ -88,7 +90,7 @@ def traverse(str, base = @base)

def association_path(str, base = @base)
base = klassify(base)
str ||= Constants::EMPTY
str ||= ''.freeze
path = []
segments = str.split(/_/)
association_parts = []
Expand Down Expand Up @@ -131,15 +133,15 @@ def ransackable_scope?(str, klass)
klass.ransackable_scopes(auth_object).any? { |s| s.to_s == str }
end

def searchable_attributes(str = Constants::EMPTY)
def searchable_attributes(str = ''.freeze)
traverse(str).ransackable_attributes(auth_object)
end

def sortable_attributes(str = Constants::EMPTY)
def sortable_attributes(str = ''.freeze)
traverse(str).ransortable_attributes(auth_object)
end

def searchable_associations(str = Constants::EMPTY)
def searchable_associations(str = ''.freeze)
traverse(str).ransackable_associations(auth_object)
end
end
Expand Down
11 changes: 5 additions & 6 deletions lib/ransack/helpers/form_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ def value(object)
RANSACK_FORM_BUILDER = 'RANSACK_FORM_BUILDER'.freeze

require 'simple_form' if
(ENV[RANSACK_FORM_BUILDER] || Ransack::Constants::EMPTY)
.match('SimpleForm'.freeze)
(ENV[RANSACK_FORM_BUILDER] || ''.freeze).match('SimpleForm'.freeze)

module Ransack
module Helpers
Expand Down Expand Up @@ -47,7 +46,7 @@ def attribute_select(options = nil, html_options = nil, action = nil)
raise ArgumentError, formbuilder_error_message(
"#{action}_select") unless object.respond_to?(:context)
options[:include_blank] = true unless options.has_key?(:include_blank)
bases = [Constants::EMPTY] + association_array(options[:associations])
bases = [''.freeze].freeze + association_array(options[:associations])
if bases.size > 1
collection = attribute_collection_for_bases(action, bases)
object.name ||= default if can_use_default?(
Expand All @@ -66,13 +65,13 @@ def attribute_select(options = nil, html_options = nil, action = nil)
def sort_direction_select(options = {}, html_options = {})
unless object.respond_to?(:context)
raise ArgumentError,
formbuilder_error_message(Constants::SORT_DIRECTION)
formbuilder_error_message('sort_direction'.freeze)
end
template_collection_select(:dir, sort_array, options, html_options)
end

def sort_select(options = {}, html_options = {})
attribute_select(options, html_options, Constants::SORT) +
attribute_select(options, html_options, 'sort'.freeze) +
sort_direction_select(options, html_options)
end

Expand Down Expand Up @@ -135,7 +134,7 @@ def predicate_select(options = {}, html_options = {})
else
only = Array.wrap(only).map(&:to_s)
keys = keys.select {
|k| only.include? k.sub(/_(any|all)$/, Constants::EMPTY)
|k| only.include? k.sub(/_(any|all)$/, ''.freeze)
}
end
end
Expand Down
16 changes: 8 additions & 8 deletions lib/ransack/helpers/form_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def initialize(search, attribute, args, params)
def name
[ERB::Util.h(@label_text), order_indicator]
.compact
.join(Constants::NON_BREAKING_SPACE)
.join(' '.freeze)
.html_safe
end

Expand All @@ -106,8 +106,8 @@ def url_options
def html_options(args)
html_options = extract_options_and_mutate_args!(args)
html_options.merge(
class: [[Constants::SORT_LINK, @current_dir], html_options[:class]]
.compact.join(Constants::SPACE)
class: [['sort_link'.freeze, @current_dir], html_options[:class]]
.compact.join(' '.freeze)
)
end

Expand Down Expand Up @@ -159,7 +159,7 @@ def detect_previous_sort_direction_and_invert_it(attr_name)
if sort_dir = existing_sort_direction(attr_name)
direction_text(sort_dir)
else
default_sort_order(attr_name) || Constants::ASC
default_sort_order(attr_name) || 'asc'.freeze
end
end

Expand All @@ -179,17 +179,17 @@ def order_indicator
end

def no_sort_direction_specified?(dir = @current_dir)
!Constants::ASC_DESC.include?(dir)
!['asc'.freeze, 'desc'.freeze].freeze.include?(dir)
end

def direction_arrow
return Constants::DESC_ARROW if @current_dir == Constants::DESC
return Constants::DESC_ARROW if @current_dir == 'desc'.freeze
Constants::ASC_ARROW
end

def direction_text(dir)
return Constants::ASC if dir == Constants::DESC
Constants::DESC
return 'asc'.freeze if dir == 'desc'.freeze
'desc'.freeze
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/ransack/nodes/condition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def inspect
]
.reject { |e| e[1].blank? }
.map { |v| "#{v[0]}: #{v[1]}" }
.join(Constants::COMMA_SPACE)
.join(', '.freeze)
"Condition <#{data}>"
end

Expand Down
6 changes: 3 additions & 3 deletions lib/ransack/nodes/grouping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def values
def respond_to?(method_id)
super or begin
method_name = method_id.to_s
writer = method_name.sub!(/\=$/, Constants::EMPTY)
writer = method_name.sub!(/\=$/, ''.freeze)
attribute_method?(method_name) ? true : false
end
end
Expand Down Expand Up @@ -114,7 +114,7 @@ def groupings=(groupings)

def method_missing(method_id, *args)
method_name = method_id.to_s
writer = method_name.sub!(/\=$/, Constants::EMPTY)
writer = method_name.sub!(/\=$/, ''.freeze)
if attribute_method?(method_name)
if writer
write_attribute(method_name, *args)
Expand Down Expand Up @@ -169,7 +169,7 @@ def inspect
]
.reject { |e| e[1].blank? }
.map { |v| "#{v[0]}: #{v[1]}" }
.join(Constants::COMMA_SPACE)
.join(', '.freeze)
"Grouping <#{data}>"
end

Expand Down
4 changes: 2 additions & 2 deletions lib/ransack/nodes/sort.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ def name=(name)
def dir=(dir)
dir = dir.downcase if dir
@dir =
if Constants::ASC_DESC.include?(dir)
if ['asc'.freeze, 'desc'.freeze].freeze.include?(dir)
dir
else
Constants::ASC
'asc'.freeze
end
end

Expand Down
4 changes: 2 additions & 2 deletions lib/ransack/predicate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def names
end

def names_by_decreasing_length
names.sort { |a,b| b.length <=> a.length }
names.sort { |a, b| b.length <=> a.length }
end

def named(name)
Expand All @@ -19,7 +19,7 @@ def named(name)

def detect_and_strip_from_string!(str)
if p = detect_from_string(str)
str.sub! /_#{p}$/, Constants::EMPTY
str.sub! /_#{p}$/, ''.freeze
p
end
end
Expand Down
8 changes: 4 additions & 4 deletions lib/ransack/search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def result(opts = {})

def build(params)
collapse_multiparameter_attributes!(params).each do |key, value|
if Constants::S_SORTS.include?(key)
if ['s'.freeze, 'sorts'.freeze].freeze.include?(key)
send("#{key}=", value)
elsif base.attribute_method?(key)
base.send("#{key}=", value)
Expand Down Expand Up @@ -92,7 +92,7 @@ def new_sort(opts = {})

def method_missing(method_id, *args)
method_name = method_id.to_s
getter_name = method_name.sub(/=$/, Constants::EMPTY)
getter_name = method_name.sub(/=$/, ''.freeze)
if base.attribute_method?(getter_name)
base.send(method_id, *args)
elsif @context.ransackable_scope?(getter_name, @context.object)
Expand All @@ -113,8 +113,8 @@ def inspect
[:base, base.inspect]
]
.compact
.map { |d| d.join(Constants::COLON_SPACE) }
.join(Constants::COMMA_SPACE)
.map { |d| d.join(': '.freeze) }
.join(', '.freeze)

"Ransack::Search<#{details}>"
end
Expand Down
4 changes: 2 additions & 2 deletions lib/ransack/translate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def self.attribute(key, options = {})
|x| x.respond_to?(:model_name)
}
predicate = Predicate.detect_from_string(original_name)
attributes_str = original_name.sub(/_#{predicate}$/, Constants::EMPTY)
attributes_str = original_name.sub(/_#{predicate}$/, ''.freeze)
attribute_names = attributes_str.split(/_and_|_or_/)
combinator = attributes_str.match(/_and_/) ? :and : :or
defaults = base_ancestors.map do |klass|
Expand Down Expand Up @@ -74,7 +74,7 @@ def self.association(key, options = {})
def self.attribute_name(context, name, include_associations = nil)
@context, @name = context, name
@assoc_path = context.association_path(name)
@attr_name = @name.sub(/^#{@assoc_path}_/, Constants::EMPTY)
@attr_name = @name.sub(/^#{@assoc_path}_/, ''.freeze)
associated_class = @context.traverse(@assoc_path) if @assoc_path.present?
@include_associated = include_associations && associated_class

Expand Down

0 comments on commit 381a83c

Please sign in to comment.