CODEWITHSUNDEEP
c#.netlistcollectionscomparison
Lukas Häfliger
Lukas Häfliger

Reputation: 526

IComparer not sorting properly

I am currently working on a project where I need to sort entries of a list of tuples according to a certain scheme. For that purpose I wrote a simple IComparer:

private class OrderComparer : IComparer<Tuple<string, DateTime, string>>
{
    public int Compare(Tuple<string, DateTime, string> x, Tuple<string, DateTime, string> y)
    {
        var yearX = x.Item1.Substring(x.Item1.Length - 2);
        var yearY = y.Item1.Substring(y.Item1.Length - 2);
        var monthX = x.Item1.Substring(x.Item1.Length - 4, 2);
        var monthY = y.Item1.Substring(y.Item1.Length - 4, 2);
        var numberX = x.Item1.Substring(1, x.Item1.Length - 5);
        var numberY = y.Item1.Substring(1, y.Item1.Length - 5);


        if (!yearX.Equals(yearY))
        {
            return Convert.ToInt32(yearX).CompareTo(Convert.ToInt32(yearY));
        }
        if (!monthX.Equals(monthY))
        {
            return Convert.ToInt32(monthX).CompareTo(Convert.ToInt32(monthY));
        }
        return Convert.ToInt32(numberX).CompareTo(Convert.ToInt32(numberY));
    }
}

the reading of yearX/Y, monthX/Y and numberX/Y works correctly as found in a debugging session.

The problem I am now facing is that it sorts correctly after the year and month but not the number. I verified, that 

return Convert.ToInt32(numberX).CompareTo(Convert.ToInt32(numberY));

returns the correct value (1 when numberX > numberY).

I call the sort method using the following code:

var dataList = data as IList<Tuple<string, DateTime, string>> ?? data.ToList();
dataList.ToList().Sort(new OrderComparer());

where data is an IEnumerable.

I am sorry for this rather simple question but I am completely stuck and I don't see any error in my implementation. Kind regards

Lukas EDIT: since sample data is needed. Here's the data the algorithm fails: B080114, B140114, B100114, B160114, B130114

this is the actual result after sorting

Upvotes: 0

Views: 363

Answers (2)

Chris
Chris

Reputation: 27599

Your problem is in this line:

dataList.ToList().Sort(new OrderComparer())

the Sort method as you know doesn't return the sorted version but operates on the original list. Here however the list it is sorting is not dataList but dataList.ToList() which is not the same item.

So it sorts dataList.ToList() in place but since you have no reference to it it is thrown away and you are left with the unmodified dataList instead.

The best fix would probably be to change the preceding line to:

var dataList = data.ToList();

So rather than checking if it is a suitable IList and so on we are just doing a ToList() to make sure we have a List so the next line can then be:

dataList.Sort(new OrderComparer());

This will now work on the correct list and hopefully thus do what you want.

Upvotes: 1

willeM_ Van Onsem
willeM_ Van Onsem

Reputation: 476584

It's hard to guess what the problem is since you don't provide sample data.

I assume you should replace

var numberX = x.Item1.Substring(1, x.Item1.Length - 5);

with:

var numberX = x.Item1.Substring(0, x.Item1.Length - 6);

(don't forget to modify the line for numberY as well). Since strings start with index 0 (and you probably only grab the last digit).

Although I would advice you to simply parse the string to a DateTime object (there is a builtin method for this, that will probably suffice), and then use the default comparable of the DateTime object. This will improve speed as well since this method will parse string objects on average O(n log n) times [and worst case O(n^2) times] instead of O(n) times

Upvotes: 2

Related Questions