4

The following code prints "false"

IEnumerable<string> x = new List<string>();
Console.WriteLine(x.Contains(null));

But this code throws an ArgumentNullException:

IEnumerable<string> x = new Dictionary<string, string>().Keys;
Console.WriteLine(x.Contains(null));

I saw this post explaining why Dictionary.ContainsKey throws if null is passed in, so I'm guessing this behavior is related. However, in the case of ContainsKey I get nice green squigglies, whereas with IEnumerable my app crashes:

enter image description here

The consuming code isn't going to know the underlying type of IEnumerable passed to it, so either we need to:

  • Not use IEnumerable.Contains() with nullable types in general or
  • Convert KeyCollection to a list before treating them as IEnumerable

Is this right, or am I missing something?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
chanban
  • 480
  • 1
  • 4
  • 19

3 Answers3

4

I assume that you want to expose the Keys property as an IEnumerable<TKey> sequence that allows searching for null. An easy way to do it is to wrap the collection in an IEnumerable<TKey> implementation, that hides the identity of the collection:

static IEnumerable<T> HideIdentity<T>(this IEnumerable<T> source)
{
    ArgumentNullException.ThrowIfNull(source);
    foreach (var item in source) yield return item;
}

Usage example:

IEnumerable<string> x = new Dictionary<string, string>().Keys.HideIdentity();

This way the LINQ Contains operator will not detect that the collection implements the ICollection<T> interface, and will follow the slow path of enumerating the collection and comparing each key using the default comparer of the TKey type. There are two downsides to this:

  1. The CPU complexity of the operation will be O(n) instead of O(1).
  2. The comparison semantics of the Dictionary<K,V>.Comparer will be ignored. So if the dictionary is configured to be case-insensitive, the Contains will perform a case-sensitive search. This might not be what you want.

A more sophisticated approach is to wrap the collection in an ICollection<TKey> implementation, that includes special handling for the null in the Contains method:

class NullTolerantKeyCollection<TKey, TValue> : ICollection<TKey>
{
    private readonly Dictionary<TKey, TValue>.KeyCollection _source;

    public NullTolerantKeyCollection(Dictionary<TKey, TValue>.KeyCollection source)
    {
        ArgumentNullException.ThrowIfNull(source);
        _source = source;
    }

    public int Count => _source.Count;
    public bool IsReadOnly => true;
    public bool Contains(TKey item) => item == null ? false : _source.Contains(item);
    public void CopyTo(TKey[] array, int index) => _source.CopyTo(array, index);
    public void Add(TKey item) => throw new NotSupportedException();
    public bool Remove(TKey item) => throw new NotSupportedException();
    public void Clear() => throw new NotSupportedException();
    public IEnumerator<TKey> GetEnumerator() => _source.GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

static NullTolerantKeyCollection<TKey, TValue> NullTolerant<TKey, TValue>(
    this Dictionary<TKey, TValue>.KeyCollection source)
{
    return new NullTolerantKeyCollection<TKey, TValue>(source);
}

Usage example:

IEnumerable<string> x = new Dictionary<string, string>().Keys.NullTolerant();

This way the resulting sequence will preserve the performance and behavior characteristics of the underlying collection.

You mentioned a third option in the question: converting the collection to a List<T> with the ToList LINQ operator. This will create a copy of the keys, and will return a snapshot of the keys at the time the ToList was called. It might be a decent option in case the dictionary is frozen, and the number of keys is small.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • 1
    +1 for considering performance implications, something I wasn't really thinking about. I'll accept this answer since it provides some good workarounds. I was hoping for a fix that took the form of a compiler error or warning like what @etienne was referring to, but seems like there isn't. – chanban Apr 20 '23 at 06:56
3

The reason behind this behaviour is that Enumerable.Contains<IEnumerable<TSource>>(this IEnumerable<TSource> source, TSource value) has a shortcut when source implements ICollection<TSource>. In these cases it just invokes the Contains method of source. See the reference source for Enumerable.Contains.

The Keys property of the dictionary is implemented by a KeyCollection (reference source). Internally, its Contains method includes validation for a non-null key, which you are running into.

If you want to use the non-shortcut linq method for Contains you should be able to always call it by supplying a comparer parameter in the other overload of this method - even if the comparer is null.

moreON
  • 1,977
  • 17
  • 19
  • 2
    Thanks for the detailed explanation and links, this helped me understand the underlying issue. Interesting point about using the overload, although it would look strange to do this in practice, and still requires the developer to know about this specific nuance. So I'd opt for modifying the collection over modifying which overload of `Contains` I call – chanban Apr 20 '23 at 06:44
1

You could do either of those, but I recommend a third method: enable nullable reference types for your project. That way, you can't pass null to IEnumerable<string>.Contains, because string does not allow nulls. It's self documenting, how about that.

And if you treat warnings as error, you'll get more than green squigglies, you'll get actual compiler errors, which means no possiblity of crashes.

Etienne de Martel
  • 34,692
  • 8
  • 91
  • 111
  • 1
    I posted a screenshot in the question which was from an interactive notebook with `#nullable enable`, which I believe means I already have that turned on. `IEnumerable.Contains` seems perfectly happy to accept null. I'm using dotnet 6.0.401, not sure if that makes a difference – chanban Apr 20 '23 at 06:32