Skip to content
Permalink
Browse files

Include `Enumerable` in `ActiveRecord::Relation`

After discussing, we've decided it makes more sense to include it. We're
already forwarding every conflicting method to `to_a`, and there's no
conflation of concerns. `Enumerable` has no mutating methods, and it
just allows us to simplify the code. No existing methods will have a
change in behavior. Un-overridden Enumerable methods will simply
delegate to `each`.

[Sean Griffin & bogdan]
  • Loading branch information...
sgrif committed Jun 19, 2015
1 parent 7d14bd3 commit b644964b2b555798fc4b94d384b98438db863b3f
@@ -1,3 +1,7 @@
* Include the `Enumerable` module in `ActiveRecord::Relation`

*Sean Griffin & bogdan*

* Use `Enumerable#sum` in `ActiveRecord::Relation` if a block is given.

*Sean Griffin*
@@ -15,6 +15,7 @@ class Relation

VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS + CLAUSE_METHODS

include Enumerable
include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain, Delegation

attr_reader :table, :klass, :loaded, :predicate_builder
@@ -275,38 +276,26 @@ def empty?

# Returns true if there are no records.
def none?
if block_given?
to_a.none? { |*block_args| yield(*block_args) }
else
empty?
end
return super if block_given?
empty?
end

# Returns true if there are any records.
def any?
if block_given?
to_a.any? { |*block_args| yield(*block_args) }
else
!empty?
end
return super if block_given?
!empty?
end

# Returns true if there is exactly one record.
def one?
if block_given?
to_a.one? { |*block_args| yield(*block_args) }
else
limit_value ? to_a.one? : size == 1
end
return super if block_given?
limit_value ? to_a.one? : size == 1
end

# Returns true if there is more than one record.
def many?
if block_given?
to_a.many? { |*block_args| yield(*block_args) }
else
limit_value ? to_a.many? : size > 1
end
return super if block_given?
limit_value ? to_a.many? : size > 1
end

# Scope all queries to the current scope.
@@ -70,12 +70,9 @@ def maximum(column_name)
# +calculate+ for examples with options.
#
# Person.sum(:age) # => 4562
def sum(*args, &block)
if block_given?
to_a.sum(&block)
else
calculate(:sum, *args)
end
def sum(*args)
return super if block_given?
calculate(:sum, *args)
end

# This calculates aggregate values in the given column. Methods for count, sum, average,
@@ -62,11 +62,8 @@ module FinderMethods
# Person.where(name: 'Spartacus', rating: 4).pluck(:field1, :field2)
# # returns an Array of the required fields.
def find(*args)
if block_given?
to_a.find(*args) { |*block_args| yield(*block_args) }
else
find_with_ids(*args)
end
return super if block_given?
find_with_ids(*args)
end

# Finds the first record matching the specified conditions. There
@@ -242,12 +242,9 @@ def references!(*table_names) # :nodoc:
# Model.select(:field).first.other_field
# # => ActiveModel::MissingAttributeError: missing attribute: other_field
def select(*fields)
if block_given?
to_a.select { |*block_args| yield(*block_args) }
else
raise ArgumentError, 'Call this with at least one field' if fields.empty?
spawn._select!(*fields)
end
return super if block_given?
raise ArgumentError, 'Call this with at least one field' if fields.empty?
spawn._select!(*fields)
end

def _select!(*fields) # :nodoc:

4 comments on commit b644964

@bogdan

This comment has been minimized.

Copy link
Contributor

replied Jun 20, 2015

Oh god! My eyes still don't believe it is true.

@bogdan

This comment has been minimized.

Copy link
Contributor

replied Jun 20, 2015

Why didn't you remove delegators here:

delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join, to: :to_a

As in initial PR: https://github.com/rails/rails/pull/8794/files#diff-563ce9140aa399b5ba97bb2a64b7c568L14?

@sgrif

This comment has been minimized.

Copy link
Contributor Author

replied Jun 20, 2015

@mokhan

This comment has been minimized.

Copy link
Contributor

replied Jun 20, 2015

Yay! Look at all that red.

Please sign in to comment.
You can’t perform that action at this time.