Null reference exception after checking for null (checking for null doesn't work)

Question

Take a look at this code:

var categories = tokens.SelectMany(x => x.Categories);

if (categories != null)
{
    if (categories.Contains("interp")) //null ref exception
    {
        return null;
    }
}

I get Null Reference Exception when I try fo find "interp" string within categories. So it seems that "categories != null" doesn't work.

I found some suggestions (here How to check if IEnumerable is null or empty?) but they involve using .Any(). But it only makes the exception accure earlier (while using .Any()). Even ?.Any() throws the exception.

Any ideas?

Answer 1

This code will throw an NRE in categories.Contains only if the Categories property is null.

The following code will throw :

class Token
{
    public string[] Categories{get;set;}
}

var tokens=new []{new Token()};
var categories = tokens.SelectMany(x => x.Categories);
if (categories != null)
{
    if (categories.Contains("interp")) 
    {
        Console.WriteLine("Found");
    }
}

But so would

tokens.SelectMany(x => x.Categories).ToArray();

The thing that actually throws is the nested iterator inside SelectMany, not ToArray() or Contains. The stack trace for that exception is :

at System.Linq.Enumerable.<SelectManyIterator>d__17`2.MoveNext()
at System.Linq.Enumerable.Contains[TSource](IEnumerable`1 source, TSource value, IEqualityComparer`1 comparer)
at UserQuery.Main()

SelectMany will try to iterate over each Categories entry, find that the property is actually null and throw.

The quick fix is to add a Where before SelectMany to eliminate null Categories :

var categories = tokens.Where(x=>x.Categories!=null).SelectMany(x => x.Categories);

The real solution is to ensure Categories is never empty - it should be initialized to an empty array, list, whatever upon construction. When reassigned, it should never be set to null.

This example sets the _categories field to new string[0] even if a caller passes null to Categories

class Token
{
    string[] _categories=new string[0];
    public string[] Categories{
        get => _categories;
        set => _categories = value??new string[0];
    }

}

With that, Where(x=>x.Categories !=null) is no longer necessary

Answer 2

When working with collections and IEnumerable<T> avoid using null; if you have nothing to return, return an empty collection (not null).

In your particular case SelectMany will never return null, but empty collection, that's why categories != null check is useless, and you have to check tokens instead

if (null != tokens)
  // Where(x => x != null) - to be on the safe side if x == null or x.Categories == null
  if (tokens
       .Where(x => x != null && x.Categories != null)
       .SelectMany(x => x.Categories)
       .Contains("interp"))
    return null;

However, constant checking for null makes code being unreadable, that's why try check for null once:

// if tokens is null change it for an empty collection
tokens = tokens ?? new MyToken[0];

...

if (tokens 
      .Where(x => x != null && x.Categories != null)
      .SelectMany(x => x.Categories)
      .Contains("interp"))
    return null;