Array.Sort with Custom IComparer Interface

Question

I have written the following class that implements IComparer:

class CustomComparer : IComparer<string>
{
    public int Compare(string x, string y)
    {
        int intX;
        int intY;

        if (int.TryParse(x, out intX) && int.TryParse(y, out intY)) //both numbers
        {
            intX.CompareTo(intY);
        }
        else if (!int.TryParse(x, out intX) && !int.TryParse(y, out intY)) //both letters
        {
            x.CompareTo(y);
        }
        else if (!int.TryParse(x, out intX) && int.TryParse(y, out intY)) //first is a letter, second a number
        {
            return -1;
        }
        else if (int.TryParse(x, out intX) && !int.TryParse(y, out intY)) //first is a number, second is a letter
        {
            return 1;
        }

        return 0;
    }
}

I am not sure whether I have done everything right but I am currently unable to call it. In my program I have an ArrayList I am trying to call it:

CustomComparer c = new CustomComparer()
myArrayList.Sort(c);

What am I doing wrong?

Answer 1

ArrayList is non-generic collection and it required non-generic IComparer as Sort method parameter.

You should either change the collection to be generic one, like List<string> or string[] or implement IComparer instead of IComparer<string>.

Answer 2

What am I doing wrong?

You are doing at least seven things wrong.

First, you are missing two return statements.

Second, since you did not notice that you are missing two return statements, odds are good that you are missing test cases that would exercise those code paths.

Third, DRY out your code. Don't Repeat Yourself. Rewrite this code so that you call int.TryParse twice, not eight times.

Fourth, never use ArrayList in code that was written after, let's say 2005. Use List<string>.

Fifth, the return 0 is unreachable. Unreachable code is a bad practice. Rewrite the method so that every line is reachable.

Sixth, ArrayList.Sort doesn't take an IComparer<string>

Seventh, comparers are required to handle nulls. It's a good idea to handle thoses cases explicitly so that you don't dereference them accidentally. Traditionally the way to go is to say that nulls are smaller than everything else.

Eighth, though it is not wrong to return any old number, it is my practice when writing a comparer to always return 0, 1 or -1.

Ninth, it is a good practice to take an "early out" in the case where the strings are reference equals. That case is extremely fast so if you can take it, you should.

I would be inclined to write this code like this:

static int? MyParse(string s)
{
    int parsed;
    bool isValid = int.TryParse(s, out parsed);
    return isValid ? (int?)parsed : (int?) null;
}
public int Compare(string x, string y)
{
    if (ReferenceEquals(x, y)) return 0;
    if (ReferenceEquals(x, null)) return -1;
    if (ReferenceEquals(y, null)) return 1;

    // We now know that neither is null.

    int? intX = MyParse(x);
    int? intY = MyParse(y);

    if (!intX.HasValue && intY.HasValue) return -1;
    if (intX.HasValue && !intY.HasValue) return 1;

    // We now know that intX.HasValue == intY.HasValue

    int result = intX.HasValue ? intX.Value.CompareTo(intY.Value) : x.CompareTo(y);
    if (result < 0) return -1;
    if (result > 0) return 1;
    return 0;
}

Answer 3

Also use return at your top two if condition after the compareTo

Answer 4

The problem is that you're using the non-generic collection ArrayList. Its Sort() method expects a non-generic comparer, not a generic one. And since the generic IComparer<T> interface does not derive from the non-generic IComparer, passing that in will not work.

Instead what you should do to implement your comparer is derive from the generic Comparer<T> class. This class implements both the generic and non-generic IComparer interfaces so you'll only have to implement your comparison once. The comparisons will be forwarded to your actual implementation. This will allow you to use the comparer when a generic or non-generic version is required.

class CustomStringComparer : Comparer<string>
{
    public override int Compare(string x, string y)
    {
        // ...
    }
}