Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Chris Barnett December 5th, 2008 @ 07:25 AM

  -1

  The current return value for these calculations on empty sets is nil, which can be very useful:

  
  @product.reviews.average(:rating) || "not yet reviewed"
  

  With this patch applied, to get the same behaviour you'd have to do something like:

  
  @product.reviews.empty? ? "not yet reviewed" : @product.reviews.average(:rating)
  

  Less readable, less elegant, and an extra database query.

• You flagged this item as spam.
  

  dvdplm December 5th, 2008 @ 09:56 AM

  +1 to consitency Sum does (and should) return 0 for an empty assoc, so average, max and min should as well.

  @chris barnett: yours look like a special case and I also don't agree the second snippet is less readable. The first is more terse but behave on a non-intuitive 'hack'. IMHO.

• You flagged this item as spam.
  

  Chris Barnett December 5th, 2008 @ 01:33 PM

  I should also point out there is a bit of history on this. ActiveRecord actually used to return 0.0 for these calculations: http://dev.rubyonrails.org/ticke...

  @dvdplm: There is a specific test case for average returning nil over an empty set, so I don't feel I'm relying on a 'hack': http://github.com/rails/rails/tr...

• You flagged this item as spam.
  

  Ryan Bates December 5th, 2008 @ 05:18 PM

  -1, I think it should behave like the existing enumerable methods. Calling [].min or [].max returns nil in Ruby. Calling [].sum returns zero in ActiveSupport. To me this makes the most sense.

• You flagged this item as spam.
  

  Shane Mingins December 5th, 2008 @ 05:56 PM

  @chris and @ryan ... why does sum returning zero for an empty association and average nil seem normal and make sense?

  This existing Rails behaviour does seem inconsistent to me and I am trying to understand why it would be this way.

• You flagged this item as spam.
  

  Anthony Bailey December 5th, 2008 @ 07:56 PM

  1. I don't think the behavior of maximum or minimum should be altered.

  Concretely, the maximum is the member with the highest value. If there are no members, there is no such value. So [].max = nil rather than zero is correct. If you add all the members in the empty list together, on the other hand, you do indeed get zero, so [].sum = 0 is correct.

  Abstractly, it's more important for an operation to be consistent with itself than with other operations. So this kind of foldy set operation should be distributive if possible: for two sets S and T, (S + T).op should if possible be (S.op, T.op).op. If you put [].max = 0, this will fail for e.g. S = [] and T = [ -1 ].

  A symmetric argument to that for max applies for min.

  I wouldn't mind a change to make the average of an empty set to return zero, since that does fulfil the identity above. But given the natural implementation of S.avg is S.sum / S.size, and this is undefined for [].sum / [].size = 0 / 0, I can see why the existing behavior exists.

  (Ruby and ActiveSupport don't define Array#avg or similar. Maybe the lack of a clear winner for this edge case is why?)

• You flagged this item as spam.
  

  Ryan Bates December 5th, 2008 @ 10:17 PM

  @Shane, I'm okay with average returning zero. It's mainly min/max that I have a problem with. I think Anthony explained it well. Zero plays no part in the min/max calculation. But you could argue it plays a part in "sum" because it could be the initial value.

• 

  Pratik December 6th, 2008 @ 12:12 AM

  • State changed from “new” to “wontfix”

  Agree with all the negative concerns here.

• You flagged this item as spam.
  

  Will Bryant February 10th, 2009 @ 02:25 AM

  I agree, from a maths/accounting perspective, min, max, and avg on no rows should return nil. Sum is defined on the empty set because there is an additive identity (0). There is no constant identity for min, max, and avg operations.

  -1, sorry :|.