Misleading description of Iterable.sizeCompare()

[leventov]

https://github.com/scala/scala/blob/b0e1ad01704fe7bace4b236c1cc48fcdbd04b081/src/library/scala/collection/Iterable.scala#L263-L265

It looks to me that it should say: "The method should be overridden if computing size is not cheap and knownSize returns -1."

[Jasper-M]

No, if computing size is cheap then calling size and comparing it with otherSize is cheaper than O(size min otherSize), so if size is cheap then sizeCompare should be overriden to just call size.

On the other hand, if size is cheap then knownSize should not return -1, so that advice seems a bit silly. If you know that computing size is cheap just override knownSize and then you don't have to override sizeCompare and you get a bunch of other improvements for free.

[leventov]

Yes, I was not sure whether the advice about overriding should be when size() is cheap or not cheap, but the two parts of the sentence are contradictory.

[som-snytt]

Maybe instead of cheap it should say cheaper.

List has a custom lengthCompare but not knownSize.

[Hamza-Meknassi]

In the documentation of iterable.scala in the scala collection almost in the part that defines us how to change an 'Iterable[A] (Line: 156-161)
in a collection of type 'c' it is defined that this process can be solved in a strict way and not strict way. and after it’s written ("In other words, this methods defines the evaluation model of the collection"), what I didn’t understand is ("this methods defines") 'Cause I think it has to be these instead of this if it means both methods if it means one method it has to be method and define without s.
https://github.com/scala/scala/blob/b0e1ad01704fe7bace4b236c1cc48fcdbd04b081/src/library/scala/collection/Iterable.scala#L263-L265

[NthPortal]

It looks to me that it should say: "The method should be overridden if computing size is not cheap and knownSize returns -1."

It is definitely quite confusingly worded, so let me step through the possibilities:

• computing size is cheap and knownSize returns the size
  • the method is already optimised to handle this efficiently
• computing size is not cheap and knownSize returns -1
  • unless there's some really weird situation, you can't really do better than the default implementation which iterates over the collection
• computing size is not cheap and knownSize returns the size
  • your implementation of knownSize is wrong/bad, and ought to return -1
• computing size is cheap and knownSize returns -1
  • since knownSize returns -1, it will iterate over the collection; however, you can do better since computing the size is cheap
  • it would be better to just change knownSize to return the size, but perhaps you can't for some reason?

The comment is indeed correct, though certainly confusing. Perhaps it should be changed to also suggest changing knownSize to return the size instead?