docstring for clojure.core/counted? should be more specific

Question

The docstring for {{counted?}} currently says:

bq. Returns true if coll implements count in constant time

This tempts the user into thinking they can use this function to determine whether or not calling {{count}} on any collection is a constant-time operation, when in fact it only reflects whether or not an object implements the {{clojure.lang.Counted}} interface. Since {{count}} special-cases a handful of platform types, there are common cases such as Arrays and Strings that are constant time but will return false from {{counted?}}.

*Proposed:*

bq. Returns true if coll, a Clojure collection, implements count in constant time. Note that this function will return false for host types even if the count function can return their size in constant time (as with arrays and strings).

Answer 1

Comment made by: gfredericks

Attached CLJ-1607-p1.patch with my first draft of a better docstring.

Answer 2

Comment made by: gfredericks

What would be the most accurate language to describe the exceptions? I used "some collections" in the first patch but perhaps "native collections" or "host collections" would be more helpful?

Answer 3

Comment made by: alexmiller

While I understand where you're coming from, I think the intent of "counted?" is not to answer the question "is this thing countable in constant time" for all possible types, but specifically for collections that participate in the Clojure collection library. This includes both internal collections like PHM, PHS, PV, etc but also external collections that mark their capabilities using those interfaces.

I believe count handles more cases than just collections that are counted in constant time (like seqs) so is not intended to be symmetric with counted?.

Answer 4

Comment made by: gfredericks

Sure, I wasn't suggesting changing what the function does -- just changing the docstring to make it less likely to be misleading.

Answer 5

Comment made by: gfredericks

What about this sort of wording?

Returns true if coll, a Clojure collection, implements count in constant time. Note that this function will return false for host types even if the count function can return their size in constant time (as with arrays and strings).

Answer 6

Comment made by: alexmiller

I think it's unlikely to pass vetting, but that's just my guess.

Answer 7

Comment made by: gfredericks

I'm trying to figure out where the disagreement is here; are you arguing any of these points, or something different?

1. The docstring is not likely to confuse people by making them think it gives meaningful responses for host collections
2. It's not a problem for us to solve if the docstring confuses people
3. It is a problem we should solve, but the changes I've suggested are a bad solution

Answer 8

Comment made by: alexmiller

In general, the docstrings prefer concision and essence over exhaustive cases or examples. My suspicion is that the docstring says what Rich wants it to say and he would consider the points you've added to be implicit in the current docstring, and thus unnecessary. Specifically, "coll" is used pretty consistently to mean a Clojure collection (or sequence) across all of the docstrings. And there is an implicit else in the docstring that counted? will return false for things that are not Clojure collections. The words that are there (and not there) are carefully chosen.

I agree with you that more words may be necessary to describe fully what to expect from this or any other function in core. My experience from seeing Rich's response on things like this is that he may agree with that too, but he would prefer it to live somewhere outside the doc string in reference material or other sources. Not to say that we don't update docstrings, as that does happen pretty regularly; I just don't think this one will be accepted. I've asked Stu to give me a second set of eyes too.

Answer 9

Comment made by: gfredericks

That was helpful detail, thanks!

Answer 10

Comment made by: arrdem

I think this one is fine as-is, because the docstring for count explicitly notes "Also works on ..." which are implied not to be counted?.

Answer 11

Reference: https://clojure.atlassian.net/browse/CLJ-1607 (reported by gfredericks)