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

ValueTuple.CompareTo documentation is wrong #2956

Closed
int19h opened this issue Aug 21, 2017 · 4 comments
Closed

ValueTuple.CompareTo documentation is wrong #2956

int19h opened this issue Aug 21, 2017 · 4 comments
Assignees
Labels
⌚ Not Triaged Not triaged
Milestone

Comments

@int19h
Copy link

int19h commented Aug 21, 2017

https://docs.microsoft.com/en-us/dotnet/api/system.valuetuple.compareto

The current description of the method is:

Returns 0 if other is a ValueTuple instance and 1 if other is null.

This is obviously wrong for many reasons, starting with the fact that other cannot be null since it's a value type. And, of course, tuples compare according to the values of their elements, not just type. Furthermore, this method - like IComparable.CompareTo, to which it corresponds - returns -1, 0 or 1 depending on the relative ordering, not just 0 and 1.

Reading the source code of ValueTuple shows that it does what you'd expect it to do, namely, lexicographical ordering, comparing the elements left to right using the default comparer, until it finds the first pair that is not equal; the result of that comparison is then returned. If all elements are equal, then tuples are considered equal.

However, it's not clear whether this is an explicit design guarantee, or just an artifact of implementation. By implementing IComparable, tuples commit to providing some total ordering, but not necessarily this way of doing it. Given the nature of tuples, this common sense implementation should probably be the guarantee - it's useful in many scenarios, such as e.g. comparing coordinates, versions, and any other case where lexicographical comparison is appropriate.

But as it is an API design question, the maintainers of the component should clarify the intent. @stephentoub?

@rpetrusha
Copy link
Contributor

Thanks for noticing the error and reporting, @int19h. The documentation is indeed incorrect, although actually ValueTuple.CompareTo (the comparison of two 0-tuples) always returns 0. I've opened PR #2976 to correct the documentation.

@int19h
Copy link
Author

int19h commented Aug 23, 2017

Ah, I see. I suppose I expected to see the general rule for how CompareTo works for tuples of any size on that page.

Looking at pages for 2-tuple etc, I see that they say that there's a relative position, which they define in terms of "this instance precedes/follows other in sort order". But they still don't define what that sort order actually is.

@int19h
Copy link
Author

int19h commented Aug 23, 2017

@rpetrusha
Copy link
Contributor

@int19h, you're certainly correct that the documentation is lacking in detail about the comparison. I'll open an issue to add it to the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⌚ Not Triaged Not triaged
Projects
None yet
Development

No branches or pull requests

5 participants