I am writing an IComparer<T> implementation by deriving from the Comparer<T> class, as recommended by MSDN. For example:
public class MyComparer : Comparer<MyClass>
{
private readonly Helper _helper;
public MyComparer(Helper helper)
{
if (helper == null)
throw new ArgumentNullException(nameof(helper));
_helper = helper;
}
public override int Compare(MyClass x, MyClass y)
{
// perform comparison using _helper
}
}
However, under this approach, the MyComparer class inherits the Default and Create static members from the Comparer<T> class. This is undesirable, since the implementation of the said members is unrelated to my derived class, and may lead to misleading behaviour:
// calls MyClass.CompareTo or throws InvalidOperationException
MyComparer.Default.Compare(new MyClass(), new MyClass());
My comparer cannot have a default instance due to the required Helper argument, nor can it initialize itself from a Comparison<T>, so I cannot hide the inherited static members with meaningful implementations.
What is the recommended practice for such situations? I'm considering three options:
Implement
IComparer<T>manually, rather than deriving fromComparer<T>, so as to avoid inheriting the said static membersLeave the inherited static members in place, and assume consumers will know not to use them
Hide the inherited static members with new implementations that throw
InvalidOperationException:public static new Comparer<MyClass> Default { get { throw new InvalidOperationException(); } } public static new Comparer<MyClass> Create(Comparison<MyClass> comparison) { throw new InvalidOperationException(); }
IComparer<T>interface and you would only have to copy/paste theIComparer.Comparemethod instead of subclassing a class that clearly was built for something else? – JorrieComparer<MyClass>.Defaultas it will call the property in base class. Btw I see no reason to inherit fromComparer<T>. – LavalleeComparer<T>: "We recommend that you derive from theComparer<T>class instead of implementing theIComparer<T>interface, because theComparer<T>class provides an explicit interface implementation of theIComparer.Comparemethod and theDefaultproperty that gets the default comparer for the object." The former use-case fits my bill; the latter doesn't. – KaylynComparerBase<T>that providesIComparer.Comparebut notDefault. (I need to have severalIComparer<T>implementations, and those 10 lines of code quickly add up.) – KaylynMyComparer.Defaultanyway, andComparer<MyClass>.Defaultreturns a different instance. – Upmostprotectedmethod, and thepublicis sealed. If the latter was the case, you could argue it might be helpful, but this is borderline ridiculous. – UpmostIComparer<T>implementation. – KaylynComparerBase<T>. I thought your problem was that your "perfect" implementation ofComparerBase<T>.IComparer.Comparerwasnt "perfect" enough. To solve that problem, looking at coreclr is ok. – JorrieIComparer.Compare(object,object)implementation which calls the overriddenCompare(T,T). – Kaylynnull-checks in the publicCompare(T, T)method, also? Because the only timeCompare(object, object)will be called if you use non-generic classes, or cast to plainIComparable? And, that in that case, implementing the non-generic method boils down to simply calling the generic one? So, the end result is that by implementing the abstractComparer<T>, you actually end up with twice the amount of null-checking when theIComparer.Compare(object, object)method is called (which is btw, rarely, since .NET 2 was released). – UpmostArgumentExceptionfor incorrect types, and then call the genericCompare. – KaylynIComparer<T>), you ended up overriding an abstract class, just because it also implements an additional single-method non-generic interface, which you didn't need in the first place. – UpmostCompare(object,object)andCompare(T,T)call a common private method that skips the null checks, but that would incur an extra method call in theCompare(T,T)hot path. And I agree that the need of implementingICompareris questionable. – Kaylyn