0

A previous question discusses IEnumerable and the convention of using empty collections instead of null valued ones. It is a good practice as it does away with many mistake-prone null checks.

But the answers don't quite address one of the cases. Many times I'm forced to deal with null values, specifically when arrays are returned from foreign methods. An example:

(foreignObj.GetPeople() ?? Enumerable.Empty<Person>())
  .Where(p => p.Name != "John")
  .OrderBy(p => p.Name)
  .Take(4);

I've written a helper method which does improve readability somewhat.

public class SafeEnumerable
{
    public static IEnumerable<T> From<T>(T[] arr)
    {
        return arr ?? Enumerable.Empty<T>();
    }
}

Resulting in:

SafeEnumerable.From(foreignObj.GetPeople())
  .Where(p => p.Name != "John")
  .OrderBy(p => p.Name)
  .Take(4);

I don't mind this, but I'm looking for better ideas. It seems like I'm adding something that should be there already.

Community
  • 1
  • 1
CÅdahl
  • 442
  • 3
  • 13
  • 5
    In the specific case of `DirectoryInfo.GetFiles()`, you should not even need a null check or the `??` operator. That method isn't known to return null if no files are found, it returns an empty `FileInfo[]` array. So it's covered by the accepted answer to your linked question too. – BoltClock Sep 21 '11 at 08:34
  • 1
    I 've done the same, only as an `EmptyIfNull` extension method: `foo.EmptyIfNull()....` – Jon Sep 21 '11 at 08:36
  • Thanks for the correction, I've edited the question to include a fictive example instead. – CÅdahl Sep 21 '11 at 08:44
  • Related: http://stackoverflow.com/questions/2831439/shorthand-for-nested-null-checking-c#answer-2831475 – Merlyn Morgan-Graham Sep 21 '11 at 08:54

4 Answers4

2

The problem locates where you got the collection(IEnumerable<T>). If you are always busy with checking for null values of a collection, you should consider to modify the source. For example:

public User GetUser(long id) { }
public List<User> GetUsers(long companyId) { }

The first method makes sense if it returns a null when no user is found, a null return value means not found. But the second method, in my opinion, should never return a null in any normal circumstance. If no users found, an empty list should be returned, instead of a null value, which means something of the program is incorrect. And the given example in your question, I don't believe directoryInfo.GetFiles("*.txt") returns null if no txt file is found, instead it should return an empty collection.

Cheng Chen
  • 42,509
  • 16
  • 113
  • 174
  • Thanks, I was mistaken about GetFiles so modified the question which is general anyway. However, it is specifically about foreign methods that can't be changed. – CÅdahl Sep 21 '11 at 08:46
  • @CÅdahl: I think I'm answering the question you are asking :) – Cheng Chen Sep 21 '11 at 08:47
  • I agree with the list part, but I think `GetUser` should get a user, or die trying. If you want to write a getter-style method that fails without throwing an exception, it should have the signature `public bool TryGetUser(long id, out User result) { }`. – Merlyn Morgan-Graham Sep 21 '11 at 08:48
  • @DannyChen: Hm. Your answer is to modify the source, which I can not do (since it's a foreign method/class/interface). I agree that it's better to return an empty collection, but other than explicitly wrapping the foreign interface this isn't possible. Perhaps that **would** be cleaner? – CÅdahl Sep 21 '11 at 08:51
2

I have created a series of extension methods for IEnumerable, the first of which being EmptyIfNull

eg.

public static class EnumerableExtensions {

  public static IEnumerable<T> EmptyIfNull<T>(this IEnumerable<T> collection) {
    return collection ?? Enumerable.Empty<T>();
  }

}

this allows me to do

var q = foreignObj.GetPeople().EmptyIfNull().Where(p=>p.Name != "John").OrderBy(p => p.Name).Take(4);   

I have then added "Safe" extensions so I can make the code a little shorter to type/easier to read

e.g.

 public static IEnumberable<T> SafeWhere<T>(this collection<T> source,Func<T,bool> predicate) {
   return source==null ? Enumerable.Empty<T>() : source.Where(predicate);
 }

giving

var q = foreignObj.GetPeople().SafeWhere(p=>p.Name != "John").OrderBy(p => p.Name).Take(4);   
Bob Vale
  • 18,094
  • 1
  • 42
  • 49
  • I like this, with the same reservation as LukeH's answer. Also, the Safe-prefixed wrappers seem very arbitrary. – CÅdahl Sep 21 '11 at 09:27
1

If you can't modify the source to correct the method that returns null then your approach makes sense.

You could perhaps make it an extension method so that it can be used in a more idiomatic, LINQy way:

var query = foreignObj.GetPeople()
                      .AsNonNullEnumerable()
                      .Where(p => p.Name != "John")
                      .OrderBy(p => p.Name)
                      .Take(4);

// ...

public static class EnumerableExtensions
{
    public static IEnumerable<T> AsNonNullEnumerable<T>(this IEnumerable<T> source)
    {
        return source ?? Enumerable.Empty<T>();
    }
}
LukeH
  • 263,068
  • 57
  • 365
  • 409
  • It's okay technically, but seems inconsistent. While calling an extension method on a null value **can** work, it does not for many extension methods in LINQ (they decide to throw). That's the premise of the question, actually. :) – CÅdahl Sep 21 '11 at 09:25
-1

Unfortunately I don't think that there is anything built-in for this. Unless you repeat yourself:

foreach(var item in (GetUsers() ?? new User[0])) // ...

A slightly 'better' implementation (taking example from what the C# compiler generates for the yield return sytnax), which is marginally less wastefuly with memory:

class SafeEnumerable<T> : IEnumerable<T>, IEnumerator<T>
{
    private IEnumerable<T> _enumerable;
    public SafeEnumerable(IEnumerable<T> enumerable) { _enumerable = enumerable; }

    public IEnumerator<T> GetEnumerator()
    {
        if (_enumerable == null)
            return this;
        else
            return _enumerable.GetEnumerator();
    }
    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { return GetEnumerator(); }

    public T Current { get { throw new InvalidOperationException(); } }
    object System.Collections.IEnumerator.Current { get { throw new InvalidOperationException(); } }

    public bool MoveNext() { return false; }
    public void Reset() { }
    public void Dispose() { }
}
Jonathan Dickinson
  • 9,050
  • 1
  • 37
  • 60