Skip to content
This repository
Browse code

Minor optimization and code cleanup in query_methods.

- Use symbols rather than strings where possible to avoid extra object construction
- Use destructive methods where possible to avoid extra object construction
- Use array union rather than concat followed by uniq
- Use shorthand block syntax where possible
- Use consistent multiline block styles, method names, method parenteses style, and spacing
  • Loading branch information...
commit 1e583f81d209ef097e040a6a7f2145ab717153b6 1 parent 5704e3c
Eugene Gilburg egilburg authored

Showing 1 changed file with 45 additions and 38 deletions. Show diff stats Hide diff stats

  1. +45 38 activerecord/lib/active_record/relation/query_methods.rb
83 activerecord/lib/active_record/relation/query_methods.rb
@@ -119,14 +119,15 @@ def create_with_value # :nodoc:
119 119 #
120 120 # User.includes(:posts).where('posts.name = ?', 'example').references(:posts)
121 121 def includes(*args)
122   - check_if_method_has_arguments!("includes", args)
  122 + check_if_method_has_arguments!(:includes, args)
123 123 spawn.includes!(*args)
124 124 end
125 125
126 126 def includes!(*args) # :nodoc:
127   - args.reject! {|a| a.blank? }
  127 + args.reject!(&:blank?)
  128 + args.flatten!
128 129
129   - self.includes_values = (includes_values + args).flatten.uniq
  130 + self.includes_values |= args
130 131 self
131 132 end
132 133
@@ -137,7 +138,7 @@ def includes!(*args) # :nodoc:
137 138 # FROM "users" LEFT OUTER JOIN "posts" ON "posts"."user_id" =
138 139 # "users"."id"
139 140 def eager_load(*args)
140   - check_if_method_has_arguments!("eager_load", args)
  141 + check_if_method_has_arguments!(:eager_load, args)
141 142 spawn.eager_load!(*args)
142 143 end
143 144
@@ -151,7 +152,7 @@ def eager_load!(*args) # :nodoc:
151 152 # User.preload(:posts)
152 153 # => SELECT "posts".* FROM "posts" WHERE "posts"."user_id" IN (1, 2, 3)
153 154 def preload(*args)
154   - check_if_method_has_arguments!("preload", args)
  155 + check_if_method_has_arguments!(:preload, args)
155 156 spawn.preload!(*args)
156 157 end
157 158
@@ -169,14 +170,15 @@ def preload!(*args) # :nodoc:
169 170 # User.includes(:posts).where("posts.name = 'foo'").references(:posts)
170 171 # # => Query now knows the string references posts, so adds a JOIN
171 172 def references(*args)
172   - check_if_method_has_arguments!("references", args)
  173 + check_if_method_has_arguments!(:references, args)
173 174 spawn.references!(*args)
174 175 end
175 176
176 177 def references!(*args) # :nodoc:
177 178 args.flatten!
  179 + args.map!(&:to_s)
178 180
179   - self.references_values = (references_values + args.map!(&:to_s)).uniq
  181 + self.references_values |= args
180 182 self
181 183 end
182 184
@@ -229,7 +231,9 @@ def select(*fields)
229 231 end
230 232
231 233 def select!(*fields) # :nodoc:
232   - self.select_values += fields.flatten
  234 + fields.flatten!
  235 +
  236 + self.select_values += fields
233 237 self
234 238 end
235 239
@@ -249,7 +253,7 @@ def select!(*fields) # :nodoc:
249 253 # User.group('name AS grouped_name, age')
250 254 # => [#<User id: 3, name: "Foo", age: 21, ...>, #<User id: 2, name: "Oscar", age: 21, ...>, #<User id: 5, name: "Foo", age: 23, ...>]
251 255 def group(*args)
252   - check_if_method_has_arguments!("group", args)
  256 + check_if_method_has_arguments!(:group, args)
253 257 spawn.group!(*args)
254 258 end
255 259
@@ -280,24 +284,22 @@ def group!(*args) # :nodoc:
280 284 # User.order(:name, email: :desc)
281 285 # => SELECT "users".* FROM "users" ORDER BY "users"."name" ASC, "users"."email" DESC
282 286 def order(*args)
283   - check_if_method_has_arguments!("order", args)
  287 + check_if_method_has_arguments!(:order, args)
284 288 spawn.order!(*args)
285 289 end
286 290
287 291 def order!(*args) # :nodoc:
288 292 args.flatten!
289   - validate_order_args args
  293 + validate_order_args(args)
290 294
291 295 references = args.grep(String)
292 296 references.map! { |arg| arg =~ /^([a-zA-Z]\w*)\.(\w+)/ && $1 }.compact!
293 297 references!(references) if references.any?
294 298
295 299 # if a symbol is given we prepend the quoted table name
296   - args.map! { |arg|
297   - arg.is_a?(Symbol) ?
298   - Arel::Nodes::Ascending.new(klass.arel_table[arg]) :
299   - arg
300   - }
  300 + args.map! do |arg|
  301 + arg.is_a?(Symbol) ? Arel::Nodes::Ascending.new(klass.arel_table[arg]) : arg
  302 + end
301 303
302 304 self.order_values += args
303 305 self
@@ -313,13 +315,13 @@ def order!(*args) # :nodoc:
313 315 #
314 316 # generates a query with 'ORDER BY id ASC, name ASC'.
315 317 def reorder(*args)
316   - check_if_method_has_arguments!("reorder", args)
  318 + check_if_method_has_arguments!(:reorder, args)
317 319 spawn.reorder!(*args)
318 320 end
319 321
320 322 def reorder!(*args) # :nodoc:
321 323 args.flatten!
322   - validate_order_args args
  324 + validate_order_args(args)
323 325
324 326 self.reordering_value = true
325 327 self.order_values = args
@@ -361,7 +363,7 @@ def reorder!(*args) # :nodoc:
361 363 #
362 364 # will still have an order if it comes from the default_scope on Comment.
363 365 def unscope(*args)
364   - check_if_method_has_arguments!("unscope", args)
  366 + check_if_method_has_arguments!(:unscope, args)
365 367 spawn.unscope!(*args)
366 368 end
367 369
@@ -400,8 +402,12 @@ def unscope!(*args) # :nodoc:
400 402 # User.joins("LEFT JOIN bookmarks ON bookmarks.bookmarkable_type = 'Post' AND bookmarks.user_id = users.id")
401 403 # => SELECT "users".* FROM "users" LEFT JOIN bookmarks ON bookmarks.bookmarkable_type = 'Post' AND bookmarks.user_id = users.id
402 404 def joins(*args)
403   - check_if_method_has_arguments!("joins", args)
404   - spawn.joins!(*args.compact.flatten)
  405 + check_if_method_has_arguments!(:joins, args)
  406 +
  407 + args.compact!
  408 + args.flatten!
  409 +
  410 + spawn.joins!(*args)
405 411 end
406 412
407 413 def joins!(*args) # :nodoc:
@@ -783,9 +789,10 @@ def extending(*modules, &block)
783 789 end
784 790
785 791 def extending!(*modules, &block) # :nodoc:
786   - modules << Module.new(&block) if block_given?
  792 + modules << Module.new(&block) if block
  793 + modules.flatten!
787 794
788   - self.extending_values += modules.flatten
  795 + self.extending_values += modules
789 796 extend(*extending_values) if extending_values.any?
790 797
791 798 self
@@ -816,12 +823,12 @@ def build_arel
816 823
817 824 collapse_wheres(arel, (where_values - ['']).uniq)
818 825
819   - arel.having(*having_values.uniq.reject{|h| h.blank?}) unless having_values.empty?
  826 + arel.having(*having_values.uniq.reject(&:blank?)) unless having_values.empty?
820 827
821 828 arel.take(connection.sanitize_limit(limit_value)) if limit_value
822 829 arel.skip(offset_value.to_i) if offset_value
823 830
824   - arel.group(*group_values.uniq.reject{|g| g.blank?}) unless group_values.empty?
  831 + arel.group(*group_values.uniq.reject(&:blank?)) unless group_values.empty?
825 832
826 833 build_order(arel)
827 834
@@ -870,11 +877,11 @@ def where_unscoping(target_value)
870 877 end
871 878
872 879 def custom_join_ast(table, joins)
873   - joins = joins.reject { |join| join.blank? }
  880 + joins = joins.reject(&:blank?)
874 881
875 882 return [] if joins.empty?
876 883
877   - joins.map do |join|
  884 + joins.map! do |join|
878 885 case join
879 886 when Array
880 887 join = Arel.sql(join.join(' ')) if array_of_strings?(join)
@@ -901,7 +908,7 @@ def build_where(opts, other = [])
901 908 when String, Array
902 909 [@klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))]
903 910 when Hash
904   - opts = PredicateBuilder.resolve_column_aliases klass, opts
  911 + opts = PredicateBuilder.resolve_column_aliases(klass, opts)
905 912 attributes = @klass.send(:expand_hash_conditions_for_aggregates, opts)
906 913
907 914 attributes.values.grep(ActiveRecord::Relation) do |rel|
@@ -944,7 +951,7 @@ def build_joins(manager, joins)
944 951 association_joins = buckets[:association_join] || []
945 952 stashed_association_joins = buckets[:stashed_join] || []
946 953 join_nodes = (buckets[:join_node] || []).uniq
947   - string_joins = (buckets[:string_join] || []).map { |x| x.strip }.uniq
  954 + string_joins = (buckets[:string_join] || []).map(&:strip).uniq
948 955
949 956 join_list = join_nodes + custom_join_ast(manager, string_joins)
950 957
@@ -956,13 +963,12 @@ def build_joins(manager, joins)
956 963
957 964 join_dependency.graft(*stashed_association_joins)
958 965
959   - joins = join_dependency.join_associations.map { |association|
960   - association.join_constraints
961   - }.flatten
  966 + joins = join_dependency.join_associations.map!(&:join_constraints)
  967 + joins.flatten!
962 968
963   - joins.each { |join| manager.from join }
  969 + joins.each { |join| manager.from(join) }
964 970
965   - manager.join_sources.concat join_list
  971 + manager.join_sources.concat(join_list)
966 972
967 973 manager
968 974 end
@@ -983,7 +989,7 @@ def reverse_sql_order(order_query)
983 989 when Arel::Nodes::Ordering
984 990 o.reverse
985 991 when String
986   - o.to_s.split(',').collect do |s|
  992 + o.to_s.split(',').map! do |s|
987 993 s.strip!
988 994 s.gsub!(/\sasc\Z/i, ' DESC') || s.gsub!(/\sdesc\Z/i, ' ASC') || s.concat(' DESC')
989 995 end
@@ -1000,14 +1006,15 @@ def reverse_sql_order(order_query)
1000 1006 end
1001 1007
1002 1008 def array_of_strings?(o)
1003   - o.is_a?(Array) && o.all?{|obj| obj.is_a?(String)}
  1009 + o.is_a?(Array) && o.all? { |obj| obj.is_a?(String) }
1004 1010 end
1005 1011
1006 1012 def build_order(arel)
1007   - orders = order_values
  1013 + orders = order_values.uniq
  1014 + orders.reject!(&:blank?)
1008 1015 orders = reverse_sql_order(orders) if reverse_order_value
1009 1016
1010   - orders = orders.uniq.reject(&:blank?).flat_map do |order|
  1017 + orders = orders.flat_map do |order|
1011 1018 case order
1012 1019 when Symbol
1013 1020 table[order].asc

0 comments on commit 1e583f8

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