Quantcast

CollectionAssociation shadows Enumerable#count

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

CollectionAssociation shadows Enumerable#count

xmlblog (Christian Romney)
Hi all,

I realize this behavior is by design, and in some respects the right thing to do. It also pre-dates the addition of Enumerable#count. I'm wondering, however, if it's possible/desirable to allow the caller to access the Enumerable versions under certain conditions. I originally wrote the up as an issue, but was informed this would be the better venue. I'm linking to the original issue on Github because the syntax highlighting is nice. 

https://github.com/rails/rails/issues/9132

TIA for taking the time to think this over.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CollectionAssociation shadows Enumerable#count

Rafael Mendonça França

I'm not sure but this can add some confusion.

I remember we removed the possibility to pass a block to sum and count to avoid confusion of users that may think the block will change the database query.

On Jan 31, 2013 8:12 PM, "Christian Romney" <[hidden email]> wrote:
Hi all,

I realize this behavior is by design, and in some respects the right thing to do. It also pre-dates the addition of Enumerable#count. I'm wondering, however, if it's possible/desirable to allow the caller to access the Enumerable versions under certain conditions. I originally wrote the up as an issue, but was informed this would be the better venue. I'm linking to the original issue on Github because the syntax highlighting is nice. 


TIA for taking the time to think this over.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CollectionAssociation shadows Enumerable#count

Matt Jones-15
In reply to this post by xmlblog (Christian Romney)

On Jan 31, 2013, at 1:35 PM, Christian Romney wrote:

> Hi all,
>
> I realize this behavior is by design, and in some respects the right thing to do. It also pre-dates the addition of Enumerable#count. I'm wondering, however, if it's possible/desirable to allow the caller to access the Enumerable versions under certain conditions. I originally wrote the up as an issue, but was informed this would be the better venue. I'm linking to the original issue on Github because the syntax highlighting is nice.
>
> https://github.com/rails/rails/issues/9132

In the example in the issue, wouldn't this work nearly as well:

def incomplete_submissions
  submissions.to_a.count(&:incomplete?)
end

In short, if you want the Enumerable behavior, just ask for it...

--Matt Jones

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CollectionAssociation shadows Enumerable#count

Matt Wean

I just ran into this issue, and I would actually call it a bug since it causes very unexpected behavior:

> User.count { |user| user.not_a_method! }
=> 10

So the block isn't even being evaluated. I'm fine if we don't delegate to Enumerable, but I would expect #count to at least raise an error if a block is given so people don't accidentally try to count with a predicate and get the wrong answer.


On Friday, February 1, 2013 at 10:12:36 AM UTC-8, Matt Jones wrote:

On Jan 31, 2013, at 1:35 PM, Christian Romney wrote:

> Hi all,
>
> I realize this behavior is by design, and in some respects the right thing to do. It also pre-dates the addition of Enumerable#count. I'm wondering, however, if it's possible/desirable to allow the caller to access the Enumerable versions under certain conditions. I originally wrote the up as an issue, but was informed this would be the better venue. I'm linking to the original issue on Github because the syntax highlighting is nice.
>
> <a href="https://github.com/rails/rails/issues/9132" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fissues%2F9132\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF84mL5d-hn6BvJ-_Flanxf8BlFzg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fissues%2F9132\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF84mL5d-hn6BvJ-_Flanxf8BlFzg&#39;;return true;">https://github.com/rails/rails/issues/9132

In the example in the issue, wouldn't this work nearly as well:

def incomplete_submissions
  submissions.to_a.count(&:incomplete?)
end

In short, if you want the Enumerable behavior, just ask for it...

--Matt Jones

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CollectionAssociation shadows Enumerable#count

Miles Georgi
Agreed :+1:



On Monday, May 15, 2017 at 10:18:07 PM UTC, Matt Wean wrote:

I just ran into this issue, and I would actually call it a bug since it causes very unexpected behavior:

> User.count { |user| user.not_a_method! }
=> 10

So the block isn't even being evaluated. I'm fine if we don't delegate to Enumerable, but I would expect #count to at least raise an error if a block is given so people don't accidentally try to count with a predicate and get the wrong answer.


On Friday, February 1, 2013 at 10:12:36 AM UTC-8, Matt Jones wrote:

On Jan 31, 2013, at 1:35 PM, Christian Romney wrote:

> Hi all,
>
> I realize this behavior is by design, and in some respects the right thing to do. It also pre-dates the addition of Enumerable#count. I'm wondering, however, if it's possible/desirable to allow the caller to access the Enumerable versions under certain conditions. I originally wrote the up as an issue, but was informed this would be the better venue. I'm linking to the original issue on Github because the syntax highlighting is nice.
>
> <a href="https://github.com/rails/rails/issues/9132" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fissues%2F9132\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF84mL5d-hn6BvJ-_Flanxf8BlFzg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fissues%2F9132\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF84mL5d-hn6BvJ-_Flanxf8BlFzg&#39;;return true;">https://github.com/rails/rails/issues/9132

In the example in the issue, wouldn't this work nearly as well:

def incomplete_submissions
  submissions.to_a.count(&:incomplete?)
end

In short, if you want the Enumerable behavior, just ask for it...

--Matt Jones

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CollectionAssociation shadows Enumerable#count

Omar Bohsali
Also agree with Matt Wean and Miles on this one.

On Monday, May 15, 2017 at 3:33:26 PM UTC-7, Miles Georgi wrote:
Agreed :+1:

<a href="https://lh3.googleusercontent.com/-lrgf7eUoHaM/WRorOGMkSjI/AAAAAAAAyjo/y6s8-u8i_1cXiSlAaaJzd1uzBUtrgUZBgCLcB/s1600/thumbs.jpg" style="margin-left:1em;margin-right:1em" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://lh3.googleusercontent.com/-lrgf7eUoHaM/WRorOGMkSjI/AAAAAAAAyjo/y6s8-u8i_1cXiSlAaaJzd1uzBUtrgUZBgCLcB/s1600/thumbs.jpg&#39;;return true;" onclick="this.href=&#39;https://lh3.googleusercontent.com/-lrgf7eUoHaM/WRorOGMkSjI/AAAAAAAAyjo/y6s8-u8i_1cXiSlAaaJzd1uzBUtrgUZBgCLcB/s1600/thumbs.jpg&#39;;return true;">



On Monday, May 15, 2017 at 10:18:07 PM UTC, Matt Wean wrote:

I just ran into this issue, and I would actually call it a bug since it causes very unexpected behavior:

> User.count { |user| user.not_a_method! }
=> 10

So the block isn't even being evaluated. I'm fine if we don't delegate to Enumerable, but I would expect #count to at least raise an error if a block is given so people don't accidentally try to count with a predicate and get the wrong answer.


On Friday, February 1, 2013 at 10:12:36 AM UTC-8, Matt Jones wrote:

On Jan 31, 2013, at 1:35 PM, Christian Romney wrote:

> Hi all,
>
> I realize this behavior is by design, and in some respects the right thing to do. It also pre-dates the addition of Enumerable#count. I'm wondering, however, if it's possible/desirable to allow the caller to access the Enumerable versions under certain conditions. I originally wrote the up as an issue, but was informed this would be the better venue. I'm linking to the original issue on Github because the syntax highlighting is nice.
>
> <a href="https://github.com/rails/rails/issues/9132" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fissues%2F9132\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF84mL5d-hn6BvJ-_Flanxf8BlFzg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fissues%2F9132\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF84mL5d-hn6BvJ-_Flanxf8BlFzg&#39;;return true;">https://github.com/rails/rails/issues/9132

In the example in the issue, wouldn't this work nearly as well:

def incomplete_submissions
  submissions.to_a.count(&:incomplete?)
end

In short, if you want the Enumerable behavior, just ask for it...

--Matt Jones

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CollectionAssociation shadows Enumerable#count

Jason Fleetwood-Boldt
In reply to this post by Matt Wean


I've never passed an argument to a .count method in my 9 years of writing ruby code. I would never think to do such a thing and such a thing would never occur to me (passing anything — block or otherwise — to a count method)

Here's the efficiency problem with your code & proposal:

submissions.to_a.count(&:incomplete?)

This code inherently encourages a code smell design— the objects must first be retrieved from the database before they are evaluated for incomplete?. This is slow, and won't scale.

Instead, you would do well to define incomplete as a scope and not a method and instead rewrite it like so:

submissions.incomplete.count

That will correctly use Active-Relation (A-Rel) objects to create the right SQL, something like

SELECT count(*) FROM submissions where incomplete = 1

Otherwise, you fetch all submissions from the database and then call incomplete? on each one, creating both an N+1 query problem and also a scaling problem because you have to instantiate each object (object instantiation is not fast) just to find out if it responds true or false. 

For efficiency alone, I would be :thumbs_down: on a change like this as it would encourage this kind of bad coding practices.

-Jason




On May 15, 2017, at 6:16 PM, Matt Wean <[hidden email]> wrote:

I just ran into this issue, and I would actually call it a bug since it causes very unexpected behavior:

> User.count { |user| user.not_a_method! }
=> 10

So the block isn't even being evaluated. I'm fine if we don't delegate to Enumerable, but I would expect #count to at least raise an error if a block is given so people don't accidentally try to count with a predicate and get the wrong answer.


On Friday, February 1, 2013 at 10:12:36 AM UTC-8, Matt Jones wrote:

On Jan 31, 2013, at 1:35 PM, Christian Romney wrote:

> Hi all,
>
> I realize this behavior is by design, and in some respects the right thing to do. It also pre-dates the addition of Enumerable#count. I'm wondering, however, if it's possible/desirable to allow the caller to access the Enumerable versions under certain conditions. I originally wrote the up as an issue, but was informed this would be the better venue. I'm linking to the original issue on Github because the syntax highlighting is nice.
>
> <a href="https://github.com/rails/rails/issues/9132" target="_blank" rel="nofollow" onmousedown="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fissues%2F9132\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF84mL5d-hn6BvJ-_Flanxf8BlFzg';return true;" onclick="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fissues%2F9132\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNF84mL5d-hn6BvJ-_Flanxf8BlFzg';return true;" class="">https://github.com/rails/rails/issues/9132

In the example in the issue, wouldn't this work nearly as well:

def incomplete_submissions
  submissions.to_a.count(&:incomplete?)
end

In short, if you want the Enumerable behavior, just ask for it...

--Matt Jones


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

If you'd like to reply by encrypted email you can find my public key on jasonfleetwoodboldt.com (more about setting GPG: https://gpgtools.org

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CollectionAssociation shadows Enumerable#count

Phillip Getto
I think this was merged into 5.1 (https://github.com/rails/rails/pull/24203).

- Phil

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Loading...