Home > Blogs > Don't assume too much

Don't assume too much

By  Aug 10, 2008

Topics: Programming, Windows Programming

We were discussing the design of a collection class at the office the other day, when the topic of parameter checking came up. This particular collection class has a Sort method that works just like the Sort method implemented by the .NET List generic collection. The programmer implementing the collection wanted the constructor to do some checking so that it could set a flag indicating whether the Sort method would actually work. This turns out to be a very bad idea.

To review, the parameterless List.Sort method uses the default comparer for the generic type. So if you have a List<Int32>, calling Sort will compare items by calling the Int32 implementation of IComparer.

That works fine if the class that you're stuffing into the collection implements IComparable. If the class doesn't implement the IComparable interface, then calling Sort will throw an exception.

The idea, then, was to have the constructor to check the default comparer, and set a flag--call it IsSortable--so that programmers could check the flag before calling Sort. Sounds reasonable, right? Checking the flag before making the call will prevent you from having to handle an exception.

It's a nice idea, but it doesn't work. Let me illustrate with an example.

Say you have an Employee class that's defined like this:

class Employee
{
    public string Name { get; private set; }
    public int EmployeeNumber { get; private set; }
    public Employee(string nm, int no)
    {
        Name = nm;
        EmployeeNumber = no;
    }
}

And you create a List of employees:

var Employees = new List<Employee>();

Obviously, if you add items to the list and then call Employees.Sort, thr program will throw InvalidOperationException because the Employee class does not implement IComparable

.

Here, it looks like List constructor could have examined the default comparer and determined that calling Sort would throw an exception. And in this particular case, the constructor would have been right. However, it's pretty easy to fool the constructor.

Imagine that you have two different employee types that you want to keep in the list. OldEmployee has an int employee number, and NewEmployee has a decimal employee number. There are many ways to implement such a thing. One way (not the best way, or even a particularly good way) would be like this:

abstract class Employee : IComparable
{
    public int CompareTo(object obj)
    {
        throw new NotImplementedException();
    }
}
class OldEmployee : Employee, IComparable
{
    public string Name { get; private set; }
    public int EmployeeNumber { get; private set; }
    public OldEmployee(string nm, int no)
    {
        Name = nm;
        EmployeeNumber = no;
    }
    int IComparable.CompareTo(object obj)
    {
        if (obj is OldEmployee)
        {
            var o2 = obj as OldEmployee;
            return EmployeeNumber.CompareTo(o2.EmployeeNumber);
        }
        else if (obj is NewEmployee)
        {
            return -1;
        }
        return 0;
    }
}
class NewEmployee : Employee, IComparable
{
    public string Name { get; private set; }
    public decimal EmployeeNumber { get; private set; }
    public NewEmployee(string nm, decimal no)
    {
        Name = nm;
        EmployeeNumber = no;
    }
    int IComparable.CompareTo(object obj)
    {
        if (obj is NewEmployee)
        {
            var o2 = obj as OldEmployee;
            return EmployeeNumber.CompareTo(o2.EmployeeNumber);
        }
        else if (obj is OldEmployee)
        {
            return 1;
        }
        return 0;
    }
}

In this code, Employee is an abstract class, and its implementation of IComparable.Compare just throws NotImplementedException. The OldEmployee and NewEmployee classes each derive from Employee, and also explicitly implement the IComparable interface to enforce the company-defined sorting rules: employees of the same type sort together, and all OldEmployee records are sorted before any NewEmployee records.

As I said, this isn't the best way to do things. But it's valid. Understand also that the comparison functions above aren't as robust as they should be. They don't handle null at all, and they erroneously return 0 (equal) if the object passed is not of a known type. But this is just example code--not something I'd ever put into production.

We can now add items of both types to the collection, and then sort it:

Employees.Add(new NewEmployee("Jim", 1.001))
Employees.Add(new OldEmployee("Sue", 2));
Employees.Add(new NewEmployee("Jen", 1.002));
Employees.Sort();

Note that this works, even though we didn't change the way that the Employees list is constructed. If List had code to check the default comparison function and set a flag if the function failed, then it would have told us that Sort won't work, even though it works just fine.

Be careful what you assume when you're working with polymorphism. What appears to be true when you're looking at the base type may turn out to be completely false when you take into account the derived types.

Become an InformIT Member

Take advantage of special member promotions, everyday discounts, quick access to saved content, and more! Join Today.

Other Things You Might Like

Xamarin Unleashed

Xamarin Unleashed