Bug #16026
closed`Set#count` performance issues
Description
Set#size
is O(1), but Set#count
is O(N).
I would like to add alias count size
to class Set
Is it okay?
Updated by Hanmac (Hans Mackowiak) over 4 years ago
technically Set#count
comes from Enumerable
, where it has support for this too
enum.count(item) -> int
enum.count { |obj| block } -> int
so i don't think Set should make #count
work like #size
Updated by ioquatix (Samuel Williams) over 4 years ago
I understand and I agree with your logic.
Maybe it's worth considering, #count
with no arguments can invoke #size
. What do you think?
Updated by byroot (Jean Boussier) over 4 years ago
Maybe it's worth considering, #count with no arguments can invoke #size. What do you think?
Since it's what Array does (rb_ary_count
), it would make sense to do the same in set.rb
IMHO.
Updated by ioquatix (Samuel Williams) over 4 years ago
I was okay with changing my implementation to use #size
- which is what I've done. However, I don't think there is anything wrong with optimising this use case if it doesn't add any overhead to existing use case. Because Array
does it too, it makes me think it's not a completely stupid idea.
If that's the case, maybe we should consider whether this logic belongs in Enumerable
.
e.g. Enumerable#count
-> Enumerable#size
, and Enumerable#size
is implemented by calling #each
by default. However, maybe that introduce bad behaviour/expectations about behaviour. Not sure. Enumerable#size
makes less sense to me, but it would be consistent across how it's used if we implemented as above.
Updated by knu (Akinori MUSHA) over 4 years ago
I agree that the return size if !block_given? && respond_to?(:size)
logic should belong to Enumerable. (oh, and also if item
is not given)
Updated by Hanmac (Hans Mackowiak) over 4 years ago
it might not be able to be generic in Enumerable
for example an Widget might be Enumerable with the child widgets, but its size would be [width, height]
for Set#count
class Set
def count(*args)
return size if !block_given? && args.empty?
super
end
end
Updated by ioquatix (Samuel Williams) over 4 years ago
- Status changed from Open to Rejected
Given the discussion, I'm okay if we just reject this feature. @knu (Akinori MUSHA) it's up to you if you want to implement it or not. Even thought it can make sense from one POV, the correct solution for most users is just to use #size
when it's available, rather than making #count
more complicated implementation (and potentially slower).
Ultimately, it was my fault for not understanding #count
.