Where is my IComparable Implementation going wrong?

Question

namespace SortableLists
{
    using System;
    using System.Collections.Generic;

    public class Program
    {
        private static void Main() {
            var list = new List<ListItem>
                           {
                               new ListItem {AdmissionCode = "801r", Name = "Rajesh Koothrappali", RollNumber = 54},
                               new ListItem {AdmissionCode = "892k", Name = "Leonard Leakey Hofstadter", RollNumber = 34},
                               new ListItem {AdmissionCode = "1203a", Name = "Sheldon Lee Cooper", RollNumber = 46},
                               new ListItem {AdmissionCode = "802x", Name = "Howard Wolowitz", RollNumber = 98}
                           };
            list.ForEach(x => Console.WriteLine(x.RollNumber + ","+x.Name + "," + x.AdmissionCode));

            Console.Write("\n");
            list.Sort();
            list.ForEach(x => Console.WriteLine(x.RollNumber + "," + x.Name + "," + x.AdmissionCode));

            Console.ReadKey();
        }
    }

    public class ListItem : IComparable<ListItem>
    {
        public int RollNumber { get; set; }
        public string Name { get; set; }
        public string AdmissionCode { get; set; }

        #region Implementation of IComparable<in ListItem>

        public int CompareTo(ListItem other) {
            return AdmissionCode.CompareTo(other.AdmissionCode);
        }

        #endregion
    }
}

I dont know what kind of sorting this is where Admission code 1203 Dr. Sheldon shows up at the top of the list after sorting??? i was expecting 801,802,803 and 1203... can anyone explain?

Answer 1

The numbers you are comparing are not treated as numbers but as strings! And with strings the letter '1' comes before '8', so the larger number appears first because when treated as text, the order is different.

I recommend you convert this field to an int if you wish to treat it as one.

Edit: for your edited question (field now also contains letters), you will need to write custom comparison logic to compare them in the order you desire.

For example, I imagine you want the logic to be like this:

1. Split code into number & letters.
2. Compare numbers only (as int).
3. If number parts are the same for both values, compare the rest as string.

Implement this logic (or whatever logic you really want) in your CompareTo method and you'll have your desired order.

The code for such logic might be like this:

public class ListItem : IComparable<ListItem>
{
    public int RollNumber { get; set; }
    public string Name { get; set; }
    public string AdmissionCode { get; set; }

    private static readonly char[] Numbers = new[]
    {
        '0',
        '1',
        '2',
        '3',
        '4',
        '5',
        '6',
        '7',
        '8',
        '9'
    };

    #region Implementation of IComparable<in ListItem>
    public int CompareTo(ListItem other)
    {
        // Assumes AdmissionCode is in ####ABC format,
        // with at least one number and any amount of letters.
        string myNumberPart, myRemainingPart;
        string otherNumberPart, otherRemainingPart;

        SplitAdmissionCode(AdmissionCode, out myNumberPart, out myRemainingPart);
        SplitAdmissionCode(other.AdmissionCode, out otherNumberPart, out otherRemainingPart);

        int myNumber = int.Parse(myNumberPart);
        int otherNumber = int.Parse(otherNumberPart);

        int result = myNumber.CompareTo(otherNumber);

        // Numbers are different.
        if (result != 0)
            return result;

        // Numbers are same. Use text compare for the remaining part.
        return myRemainingPart.CompareTo(otherRemainingPart);
    }

    private void SplitAdmissionCode(string code, out string numbersPart, out string remainingPart)
    {
        int lastNumberIndex = code.LastIndexOfAny(Numbers);

        numbersPart = code.Substring(0, lastNumberIndex + 1);

        if (lastNumberIndex == code.Length - 1)
            remainingPart = "";
        else
            remainingPart = code.Substring(lastNumberIndex + 1);
    }
    #endregion
}

Answer 2

You are sorting strings and not numbers. The string CompareTo does not take length into account.

EDIT (For your edited question): When sorting strings which start with numbers, CompareTo method's sorting will hardly give the expected results as all it does is alphabetizing.

Answer 3

String "1203" is less than "801". You may try to convert strings into numbers before comparison(if they represent number values essentially)

Answer 4

This is the expected functionality as others have noted.
If you want the order like 801r, 802x, 892k, 1203a do this

public class ListItem : IComparable<ListItem>
{
    public int RollNumber { get; set; }
    public string Name { get; set; }
    public string AdmissionCode { get; set; }

    public int CompareTo(ListItem other) {
        return ExtractNumbers(this.AdmissionCode).CompareTo(ExtractNumbers(other.AdmissionCode));
    }
    private int ExtractNumbers(string expr) {
        return Convert.ToInt32(String.Join(null,System.Text.RegularExpressions.Regex.Split(expr, "[^\\d]")));
    }
}

Answer 5

If you want to keep it simple:

public class ListItem : IComparable<ListItem>
{
    public int RollNumber { get; set; }
    public string Name { get; set; }
    public string AdmissionCode { get; set; }

    #region Implementation of IComparable<in ListItem>

    public int CompareTo(ListItem other)
    {
        return this.AdmissionCode.Length != other.AdmissionCode.Length
            ? this.AdmissionCode.Length.CompareTo(other.AdmissionCode.Length)
            : this.AdmissionCode.CompareTo(other.AdmissionCode);
    }

    #endregion
}