Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 3 commits into from
Closed

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

wants to merge 3 commits into from

Conversation

chrisfinne
Copy link
Contributor

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

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)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@tenderlove
Copy link
Member

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

@chrisfinne
Copy link
Contributor Author

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

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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@chrisfinne
Copy link
Contributor Author

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

@isaacsanders
Copy link
Contributor

Is this still an issue?

@chrisfinne
Copy link
Contributor Author

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

@isaacsanders
Copy link
Contributor

@tenderlove Do you still want this?

@rafaelfranca
Copy link
Member

Closed by 560aa33a47719db474d09ee7a008d7a21bcabeac

@rafaelfranca
Copy link
Member

Oooops, it is not in the rails repository yet.

@rafaelfranca rafaelfranca reopened this May 25, 2012
@isaacsanders
Copy link
Contributor

@rafaelfranca It appears to be...

@rafaelfranca
Copy link
Member

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

@isaacsanders
Copy link
Contributor

o.O @github

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants