Wrong List.Sort() behaviour

Question

I've got a List of Position objects that I retrive from EF DB context

+--------+---------------------+-------+-------+
|   ID   |      FullDate       | Year  | Month |
+--------+---------------------+-------+-------+
| 21952  | null                | 2015  | 1     |
| 21953  | null                | 2015  | 1     |
| 21954  | null                | 2015  | 1     |
| 21955  | null                | 2015  | 2     |
| 21956  | null                | 2015  | 1     |
| 21957  | null                | 2015  | 2     |
| 21958  | null                | 2015  | 3     |
| 21959  | null                | 2015  | 1     |
| 21960  | null                | 2015  | 1     |
| 21961  | null                | 2015  | 1     |
| 21962  | null                | 2015  | 2     |
| 21963  | null                | 2015  | 2     |
| 21964  | null                | 2015  | 2     |
| 21965  | null                | 2015  | 2     |
| 21966  | 01.02.2015 0:00:00  | null  | null  |
| 21967  | null                | 2015  | 2     |
| 21968  | null                | 2015  | 2     |
| 21969  | null                | 2015  | 2     |
| 21970  | null                | 2015  | 2     |
| 21971  | null                | 2015  | 3     |
| 21972  | null                | 2015  | 3     |
| 21973  | null                | 2015  | 3     |
| 21974  | null                | 2015  | 3     |
| 21975  | null                | 2015  | 3     |
| 21976  | null                | 2015  | 4     |
| 21977  | null                | 2015  | 4     |
| 21978  | null                | 2015  | 4     |
| 21979  | null                | 2015  | 4     |
| 21980  | null                | 2015  | 4     |
| 21981  | null                | 2015  | 5     |
| 21982  | null                | 2015  | 5     |
| 21984  | null                | 2015  | 6     |
| 21983  | null                | 2015  | 5     |
+--------+---------------------+-------+-------+

I've impemented a sorting like so:

positions.Sort((x, y) =>
{
    var xDate = getActualDate(x.FullDate, x.Year, x.Month);
    var yDate = getActualDate(y.FullDate, y.Year, y.Month);
    if (xDate > yDate)
    {
        return 1;
    }
    if (xDate == yDate && x.Id> y.Id)
    {
        return 1;                                              
    }                                    
    return -1;
});

Method for getting actual date is

private DateTime getActualDate(DateTime? fullDate, int? year, int? month)
{
    return fullDate.HasValue ? fullDate.Value : new DateTime(year.Value, month.Value, DateTime.DaysInMonth(year.Value, month.Value));
}

Every time i try to sort last rows are not changing even if in comparator expression return 1 nothing is changed. I've tried to debug Comparator method but get no results everything seems to work without erros exept the result of a sort :(

I've also tried to change palces 21983 and 21984 manualy and then sort the collection and got the same result. I've added some watches to look for changes in colletions and as a result I saw that those two elements of collection have changed their places — Anton, Jun 01 '15 at 14:47
You're not returning 0 if you've got matching rows, for one thing... A short but *complete* program demonstrating the problem would really help here. — Jon Skeet, Jun 01 '15 at 14:49
You comparison function is not correct, because it allows the following: `Compare(a, b) => -1` and `Compare(b, a) => -1` when they are actually equal (and you should be returning 0 in those cases). — Alex, Jun 01 '15 at 14:51
They can't be actually equal because they'll have different Id's and comparator method will compare Id's in case of equal dates `if (xDate == yDate && x.Id> y.Id) { return 1; }` — Anton, Jun 01 '15 at 14:56
Are you assigning the result of `positions.Sort` *to anything*? I don't see it there. You may want to try `var sortedPositions = positions.Sort(...` — Der Kommissar, Jun 01 '15 at 16:17
@EBrown `List.Sort` doesn't return anything - it sorts the list in-place. — D Stanley, Jun 01 '15 at 16:20
@DStanley Really? Guess I am used to `OrderBy`. Ignore my comment then. — Der Kommissar, Jun 01 '15 at 16:21

D Stanley · Accepted Answer · 2017-07-10T16:14:13.207

They can't be actually equal because they'll have different Id's and comparator method will compare Id's in case of equal dates

You still need to code an "equals" case because List.Sort will compare an item to itself at certain points in the process. Comparing an item to itself should always return 0.

Luckily, the case is easy enough to inject:

positions.Sort((x, y) =>
{
    var xDate = getActualDate(x.FullDate, x.Year, x.Month);
    var yDate = getActualDate(y.FullDate, y.Year, y.Month);
    if (xDate == yDate && x.Id == y.Id)
    {
        return 0;                                              
    }                                    
    if (xDate > yDate)
    {
        return 1;
    }
    if (xDate == yDate && x.Id> y.Id)
    {
        return 1;                                              
    }                                    
    return -1;
});

You can rearrange or logically reduce the comparisons, if you like, but the logic should be the same.

score 1 · Answer 2 · answered Jun 01 '15 at 16:16

Not sure why the sort isn't working, but this should:

var result=positions.Select(p=> new { 
  id, 
  date = p.fullDate ?? new DateTime(p.year.Value, p.month.Value, DateTime.DaysInMonth(p.year.Value, p.month.Value))
}).OrderBy(p=>p.date)
  .ThenBy(p=>p.id);

Wrong List.Sort() behaviour

2 Answers2