This project is archived and is in readonly mode.
Average, Maximum, Minimum on associations should return 0 when no rows are returned
-
Chris Barnett
-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.
-
dvdplm
+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.
-
Chris Barnett
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...
-
Ryan Bates
-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.
-
Shane Mingins
@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.
-
Anthony Bailey
- 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?)
-
Ryan Bates
@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.
-
Will Bryant
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 :|.
