Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow blocks for count with ActiveRecord::Relation like we do with sum #4003

Closed
wants to merge 3 commits into from

6 participants

chrisfinne Aaron Patterson Isaac Sanders Rafael Mendonça França Eugene Gilburg Carlos Antonio da Silva
chrisfinne

also added tests and documentation about sum allowing a block.

I've been burned too many times by thinking I'm working with an Array. If we can sum on ActiveRecord::Relations like we do with Arrays, why not count with a block as well?

persons = Person.where(:gender=>'male')
persons.count{|person| false} # should equal 0, but will equal persons.size
activerecord/lib/active_record/relation/calculations.rb
((12 lines not shown))
def count(column_name = nil, options = {})
- column_name, options = nil, column_name if column_name.is_a?(Hash)
- calculate(:count, column_name, options)
+ if block_given?
+ self.to_a.count {|*block_args| yield(*block_args)}
Aaron Patterson Owner

Can you change this to just take one argument? Normal array count only takes one arg, so I'd rather not be splatting back and forth in here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Aaron Patterson
Owner

Hi, can you just make the change to *args, and I'll apply this. Thanks.

chrisfinne

Just pushed an update to this branch to use *args

Someone just fixed something so that Account.count{false}==0 now works. Changed the test and added an example to the doc.

Removed an inaccurate documentation Note about Person.count(:all) not working.

Added a ArgumentError to pass the existing test test_count_with_too_many_parameters_raises

Eugene Gilburg egilburg commented on the diff
activerecord/lib/active_record/relation/calculations.rb
@@ -51,11 +52,25 @@ module Calculations
# Person.count('id', :conditions => "age > 26") # Performs a COUNT(id)
# Person.count(:all, :conditions => "age > 26") # Performs a COUNT(*) (:all is an alias for '*')
#
- # Note: <tt>Person.count(:all)</tt> will not work because it will use <tt>:all</tt> as the condition.
- # Use Person.count instead.
- def count(column_name = nil, options = {})
- column_name, options = nil, column_name if column_name.is_a?(Hash)
- calculate(:count, column_name, options)
+ # Examples for count with a block:

This (and other comments here) seem to use old :conditions => syntax? Isn't new one simply Person.where("age > 26"), etc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
chrisfinne

@tenderlove let me know if there's anything changes needed, e.g. cleaning up the docs as @egilburg suggests

Isaac Sanders

Is this still an issue?

chrisfinne

It seems to have dropped off the radar. The patch hasn't been applied.

Isaac Sanders

@tenderlove Do you still want this?

Carlos Antonio da Silva carlosantoniodasilva referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Carlos Antonio da Silva carlosantoniodasilva referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Carlos Antonio da Silva carlosantoniodasilva referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Rafael Mendonça França

Closed by 560aa33a47719db474d09ee7a008d7a21bcabeac

Rafael Mendonça França

Oooops, it is not in the rails repository yet.

Rafael Mendonça França rafaelfranca reopened this
Isaac Sanders

@rafaelfranca It appears to be...

Rafael Mendonça França

@carlosantoniodasilva told me that it is not. This is a crazy behavior of Github. It is only in the @carlosantoniodasilva's fork

Carlos Antonio da Silva carlosantoniodasilva referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 16, 2011
  1. chrisfinne
Commits on Dec 18, 2011
  1. chrisfinne
  2. chrisfinne
This page is out of date. Refresh to see the latest.
33 activerecord/lib/active_record/relation/calculations.rb
View
@@ -3,12 +3,13 @@
module ActiveRecord
module Calculations
- # Count operates using three different approaches.
+ # Count operates using four different approaches.
#
# * Count all: By not passing any parameters to count, it will return a count of all the rows for the model.
# * Count using column: By passing a column name to count, it will return a count of all the
# rows for the model with supplied column present.
# * Count using options will find the row count matched by the options used.
+ # * Count using a block will acts as Array#count
#
# The third approach, count using options, accepts an option hash as the only parameter. The options are:
#
@@ -51,11 +52,25 @@ module Calculations
# Person.count('id', :conditions => "age > 26") # Performs a COUNT(id)
# Person.count(:all, :conditions => "age > 26") # Performs a COUNT(*) (:all is an alias for '*')
#
- # Note: <tt>Person.count(:all)</tt> will not work because it will use <tt>:all</tt> as the condition.
- # Use Person.count instead.
- def count(column_name = nil, options = {})
- column_name, options = nil, column_name if column_name.is_a?(Hash)
- calculate(:count, column_name, options)
+ # Examples for count with a block:

This (and other comments here) seem to use old :conditions => syntax? Isn't new one simply Person.where("age > 26"), etc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ # Person.where(:conditions => "age > 26").count{|person| person.gender=='female' }
+ #
+ # Person.count{|person| person.age > 26 && person.gender=='female' }
+ def count(*args)
+ if block_given?
+ self.to_a.count {|*block_args| yield(*block_args)}
+ else
+ raise ArgumentError, "Too many arguments. No more than 2 are allowed." if args.size > 2
+ if args.blank?
+ column_name, options = nil, {}
+ elsif args.first.is_a?(Hash)
+ column_name, options = nil, args.first
+ else
+ column_name = args.shift
+ options = args.shift || {}
+ end
+ calculate(:count, column_name, options)
+ end
end
# Calculates the average value on a given column. Returns +nil+ if there's
@@ -84,6 +99,12 @@ def maximum(column_name, options = {})
calculate(:maximum, column_name, options)
end
+ # Sum operates using two different approaches:
+ # * Sum using a block will act as Array#sum
+ #
+ # Person.where('age > 100').sum{|person| person.age - 100} # => 11
+ #
+ # * Sum as a Calculation on a given column.
# Calculates the sum of values on a given column. The value is returned
# with the same data type of the column, 0 if there's no row. See
# +calculate+ for examples with options.
17 activerecord/test/cases/calculations_test.rb
View
@@ -398,6 +398,23 @@ def test_count_with_from_option
assert_equal Company.count(:type, :conditions => {:type => "Firm"}),
Company.count(:type, :conditions => {:type => "Firm"}, :from => 'companies')
end
+
+ def test_count_with_block_acts_as_array
+ accounts = Account.where('id > 0')
+ assert_equal Account.count, accounts.count{ true }
+ assert_equal 0, accounts.count{ false }
+ assert_equal Account.where('credit_limit > 50').size, accounts.count{|account| account.credit_limit > 50}
+ assert_equal Account.where('credit_limit > 50').size, Account.count{|account| account.credit_limit > 50}
+ assert_equal Account.count, Account.count{true}
+ assert_equal 0, Account.count{false}
+ end
+
+ def test_sum_with_block_acts_as_array
+ accounts = Account.where('id > 0')
+ assert_equal Account.sum(:credit_limit), accounts.sum{|account| account.credit_limit }
+ assert_equal Account.sum(:credit_limit) + Account.count, accounts.sum{|account| account.credit_limit + 1 }
+ assert_equal 0, accounts.sum{|account| 0 }
+ end
def test_sum_with_from_option
assert_equal Account.sum(:credit_limit), Account.sum(:credit_limit, :from => 'accounts')
Something went wrong with that request. Please try again.