Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should an implementation of IEqualityComparer.Equals allow null values?

I have a custom generic data structure that includes a Find method:

public bool Find(TValue value, IEqualityComparer<TValue> comparer)
{
    foreach (var x in items)
    {
        if (comparer.Equals(value, x))
            return true;
    }
    return false;
}

I got a report recently from a client who said that it causes his equality comparer to throw NullReferenceException if value is null or if one of the items in the collection is null.

My initial response was that his IEqualityComparer<T>.Equals implementation was in error because it doesn't deal gracefully with null values. But I haven't been able to find any documentation to explicitly back me up. I have some evidence to indicate that I'm right, but nothing explicit.

First, it seems silly that I'd have change that simple call to comparer.Equals to:

if (x == null)
{
    if (value == null)
        return true;
}
else if (value != null && comparer.Equals(value, x))
    return true;

Second, the documentation for Object.Equals says, among other things:

  • x.Equals(null) returns false.
  • Implementations of Equals must not throw exceptions.

That, to me, is strong evidence that IEqualityComparer<T>.Equals should gracefully handle null parameters.

Other evidence is that the documentation for IComparer.Compare says:

Comparing null with any reference type is allowed and does not generate an exception. A null reference is considered to be less than any reference that is not null.

One would expect IEqualityComparer<T>.Equals to act similarly. It's amusing to note, though, that the example given on that page will throw NullReferenceException if either parameter is null.

I've been through the documentation for Object.Equals, IEquatable<T>, IEqualityComparer<T>, and IEqualityComparer, and countless blog posts, articles, and SO questions. Nothing gives any specific guidelines about how to handle null parameters.

Do such guidelines exist? If not, what do the gurus recommend, and why?

like image 757
Jim Mischel Avatar asked Dec 12 '12 02:12

Jim Mischel


3 Answers

The closest method in the .NET framework itself after which all IEqualityComparer.Equals methods should be modeled is the static Object.Equals(object,object) method. According to the documentation, this method handles nulls gracefully. I think that this provides enough indication on the intent of the .NET designers: IEqualityComparer.Equals should handle nulls as well, and it should handle them in a similar way (i.e. treating two nulls as equal to each other).

like image 155
Sergey Kalinichenko Avatar answered Nov 08 '22 05:11

Sergey Kalinichenko


The guidelines that FxCop uses, includes a stipulation that every public method of a public type must handle null arguments, such as by throwing ArgumentNullException. In your case, given the Object.Equals you noted, then you just need to do the null test and return false - because only null is equal to null :)

This is documented here: http://msdn.microsoft.com/en-us/library/ms182182(v=VS.80).aspx

like image 3
Dai Avatar answered Nov 08 '22 06:11

Dai


Well, the EqualityComparer<T> abstract base class (not the interface, but does implement it) has some comments on its Equals methods.

For EqualityComparer<T>.Equals(T, T) the MSDN doesn't state any known exceptions are typically thrown (and the MSDN is usually pretty good about listing exceptions). Certainly passing in any class (custom or BCL) and comparing it to null doesn't throw any exceptions.

http://msdn.microsoft.com/en-us/library/ms132154.aspx

like image 2
Chris Sinclair Avatar answered Nov 08 '22 06:11

Chris Sinclair