C# Sorting – Is the IComparable Interface Outdated or Harmful?

ccomparisonsorting

`IComparable` only works one way

Let's say you have a Employee class. In one view, you want to show all Employees sorted by name – in another, by address. How are you going to achieve that? Not with IComparable, at least not in any idiomatic way.

`IComparable` has the logic in the wrong place

The interface is used by calling .Sort(). In a view showing Customer sorted by name, there is no code at all to implicate how it is going to be sorted.
On the other hand, the Customer class is assuming how it is going to be used – in this case, that it will be used in a list sorted by names.

`IComparable` is used implicitly

In comparison with the alternatives, it is very difficult to see where the comparing logic is being used – or if at all. Assuming your standard IDE and starting from the Customer class, I will have to

Search for all references to Customer
Find those references which are used in a list
Check if those lists ever have .Sort() called on them

What's probably worse, if you remove an IComparable implementation that is still being used, you get no error or warning. The only thing you will get is wrong behaviour in all places that were too obscure for you to think of.

These issues combined, plus changing requirements

The very reason I came to think about this is because it went wrong for me. I have been happily using IComparable in my application for 2 years now. Now, the requirements changed and the thing needs to be sorted in 2 different ways. It have noticed that it is no fun going through the steps described in the previous section.

The question

These issues make me think of IComparable as inferior to IComparer or .OrderBy(), to the point of not seeing any valid use case that wouldn't be served better by the alternatives.
Is it always better to use IComparer or LINQ, or are there advantages/use cases I am not seeing here?

Best Answer

IComparable has the restrictions you mentioned, that is correct. It is an interface which was already available in .NET framework 1.0, where those functional alternatives and Linq were not available. So yes, one might see it as an outdated framework element which is mainly kept for backwards compatibility.

However, for lots of simple data structures, one way of sorting is probably enough or natural. For these cases, having one canonical place to implement the order relationship is still a good way to keep the code DRY, instead of always repeating the same logic in every call to OrderBy all over the place.

You have been "happily using IComparable in your application for 2 years now", as you wrote, so it appears to me it served you well for a long time. When you now have to validate, change and test all calls to Sort, it may also be a sign you were doing the same kind of sorting logic in many places, which is not the fault of IComparable. So this could be an occasion to centralize more of this logic in one place, making your code more DRY.

Related Solutions

Dependency Injection – Abstract Factory Patterns in C#

It seems you're struggling trying to create objects that create objects... wouldn't you be better off just creating objects ?

Ninject allows you to do so in a very simple way :

// Ninject module
Bind<IRepository<SomeEntity>>().To<SomeEntityRepository>();

// injected class
public class SomeService
{
  public SomeService(IRepository<SomeEntity> repository)
  {
    // ...
  }
}

// get instance of injected class
var someService = kernel.Get<SomeService>();

I can't see the benefit of an abstract factory in addition to an IoC container when you're creating objects based on basic, compile-time-known constraints such as in your SomeViewFactory and SomeRepositoryFactory examples.

The dynamic nature of SomeFunctionalityFactory would be more debatable, but when you think of it, do you really need to instantiate an object with the built-in notion that it will never be able to be used by the current user ? Wouldn't it make more sense to have the object evaluate user authorization when the functionality is called ?

C# – Testing a Conversion Class

You don't have to test .NET Framework's code in your specific case, because:

You can't inherit from Convert class, since this class is static; even if you could do that, for example if Convert class weren't static:
The Convert class has no virtual methods,
There are no instance methods,
There are no abstract methods,
You cannot override static methods in a class,
You shouldn't hide methods by using new keyword anyway (and if you do, Code analysis will shout at you).

The only way you may be able to screw with existent functionality is through Reflection. If you use Reflection in order to play with the internals of the Convert class, then yes, you have to study and test the impact it may have.

Otherwise, you just have to test the new code you'll write in your custom class.

IComparable only works one way

IComparable has the logic in the wrong place

IComparable is used implicitly