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

OrderByDescending sorts incorrectly if a comparer returns int.MinValue #14793

Closed
JonHanna opened this issue Jul 3, 2015 · 0 comments
Closed
Milestone

Comments

@JonHanna
Copy link
Contributor

JonHanna commented Jul 3, 2015

int.MinValue is of course traditionally considered less than 0. It is therefore a valid "x is less than y" return value for IComparer<T>.Compare(T x, T y).

But if an implementation returns it when it is correct, then Enumerable.OrderByDescending will treat it indicating that y is greater than x.

Demonstrating test case:

private class ExtremeComparer : IComparer<int>
{
    public int Compare(int x, int y)
    {
        if (x == y)
            return 0;
        if (x < y)
            return int.MinValue;
        return int.MaxValue;
    }
}
[Fact]
public void OrderByExtremeComparer()
{
    var outOfOrder = new[] { 7, 1, 0, 9, 3, 5, 4, 2, 8, 6 };
    Assert.Equal(Enumerable.Range(0, 10), outOfOrder.OrderBy(i => i, new ExtremeComparer()));
    Assert.Equal(Enumerable.Range(0, 10).Reverse(), outOfOrder.OrderByDescending(i => i, new ExtremeComparer()));
}

The second assertion fails, as rather than the expected 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, the result is 3, 5, 1, 4, 9, 2, 0, 8, 7, 6.

JonHanna referenced this issue in JonHanna/corefx Jul 3, 2015
int.MinValue is a valid "x is less than y" result for a comparer but
OrderByDescending treats it incorrectly. Fix this.

Fix #2239
JonHanna referenced this issue in JonHanna/corefx Jul 9, 2015
int.MinValue is a valid "x is less than y" result for a comparer but
OrderByDescending treats it incorrectly. Fix this.

Fix #2239

Simplify comparison result inverting.

As per suggestion at
dotnet#2240 (comment)

Simpler expression.

As suggested at dotnet#2240 (comment)
This reduces descending fix to a single branch in all cases.

Parentheses to clarify precedence in logic.

Replace boolean ^ with !=

Logically equivalent, and arguably better known.

Silly typo in comment.
Priya91 referenced this issue in Priya91/corefx-1 Jul 10, 2015
int.MinValue is a valid "x is less than y" result for a comparer but
OrderByDescending treats it incorrectly. Fix this.

Fix #2239

Simplify comparison result inverting.

As per suggestion at
dotnet#2240 (comment)

Simpler expression.

As suggested at dotnet#2240 (comment)
This reduces descending fix to a single branch in all cases.

Parentheses to clarify precedence in logic.

Replace boolean ^ with !=

Logically equivalent, and arguably better known.

Silly typo in comment.
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.0.0-rtm milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants