Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misleading description of Iterable.sizeCompare() #11783

Open
leventov opened this issue Oct 29, 2019 · 3 comments

Comments

@leventov
Copy link

@leventov leventov commented Oct 29, 2019

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

This comment has been minimized.

Copy link
Member

@Jasper-M Jasper-M commented Oct 29, 2019

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

This comment has been minimized.

Copy link
Author

@leventov leventov commented Oct 29, 2019

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

This comment has been minimized.

Copy link

@som-snytt som-snytt commented Oct 29, 2019

Maybe instead of cheap it should say cheaper.

List has a custom lengthCompare but not knownSize.

@SethTisue SethTisue added this to the Backlog milestone Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.