While browsing the .Net Framework source code for the Enumerable class it has occurred to me that its internal EnumerableSorter class CompareKeys method used for sorting has a following line:
return descending ? -c : c;
Where c
is the result of the IComparer.Compare Method (T, T) method call, that doesn't actually force us to use only -1, 1 or 0 to denote ordering.
Taking into account that -Int32.MinValue == Int32.MinValue
due to integer overflow, it can lead to incorrect sorting, as can be proved by the following snippet:
public class Value : IComparable<Value>
{
private readonly Int32 _value;
public Value(Int32 value)
{
this._value = value;
}
public Int32 CompareTo(Value other)
{
if (other == null)
throw new ArgumentNullException(nameof(other));
var cmp = this._value.CompareTo(other._value);
if (cmp > 0)
return Int32.MaxValue;
if (cmp < 0)
return Int32.MinValue;
return 0;
}
public override String ToString()
{
return this._value.ToString();
}
}
private static void Print<T>(String header, IEnumerable<T> values)
{
Console.WriteLine(header);
foreach (var item in values)
{
Console.WriteLine(item);
}
Console.WriteLine();
}
public static void Main()
{
try
{
var notSorted = new[] { 1, 3, 2 }
.Select(i =>
new Value(i))
.ToArray();
Print("Not sorted", notSorted);
Print("Sorted by", notSorted.OrderBy(item => item));
Print("Sorted by descending", notSorted.OrderByDescending(item => item));
}
catch (Exception exc)
{
Console.WriteLine(exc);
}
Console.WriteLine("Press any key...");
Console.ReadKey(true);
}
For OrderByDescending
it produces:
Sorted by descending 3 1 2
It was expected, but it is also an obviously incorrect result.
So it seems to be a defect in the .Net, though a very unlikely to occur if CompareTo
implemented in a reasonable way. Am I right?
UPDATE:
As pointed by SLaks the issue has been known for a long time but hasn't been fixed despite all the new releases - https://connect.microsoft.com/VisualStudio/feedback/details/634949/orderbydescending-fails-in-linq-to-objects-when-a-comparer-returns-int-minvalue
As pointed by usr .Net Core has this issue fixed - https://github.com/dotnet/corefx/blob/35e03c78d89d02f2d3b4a1f8b277a35c88f45750/src/System.Linq/src/System/Linq/OrderedEnumerable.cs#L628
Seems that there isn't much of an answer to be made, so:
As pointed by SLaks the issue has been known for a long time but has never been fixed in the .NET Framework despite all the new releases (.Net 4.6.1 as of now) - https://connect.microsoft.com/VisualStudio/feedback/details/634949/orderbydescending-fails-in-linq-to-objects-when-a-comparer-returns-int-minvalue.
The only way to avoid this problem is to not return the Int32.MinValue
from CompareTo
implementations.
But as pointed by usr .Net Core has this issue fixed - https://github.com/dotnet/corefx/blob/35e03c78d89d02f2d3b4a1f8b277a35c88f45750/src/System.Linq/src/System/Linq/OrderedEnumerable.cs#L628