144

Is there a nicer way of doing the following:
I need a check for null to happen on file.Headers before proceeding with the loop

if (file.Headers != null)
{
  foreach (var h in file.Headers)
  {
   //set lots of properties & some other stuff
  }
}

In short it looks a bit ugly to write the foreach inside the if due to the level of indentation happening in my code.

Is something that would evaluate to

foreach(var h in (file.Headers != null))
{
  //do stuff
}

possible?

Eminem
  • 7,206
  • 15
  • 53
  • 95
  • 3
    You can have a look here: http://stackoverflow.com/questions/6937407/null-exception-handling-in-foreach-loop – Adrian Fâciu Jul 31 '12 at 06:34
  • http://stackoverflow.com/questions/872323/method-call-if-not-null-in-c-sharp is another idea. – weismat Jul 31 '12 at 06:36
  • 1
    @AdrianFaciu I think that's completely different. The question checks if the collection is null first before doing the for-each. Your link checks if the item in the collection is null. – rikitikitik Jul 31 '12 at 06:38
  • Similar to http://stackoverflow.com/q/3088147/80161 and http://stackoverflow.com/q/6455311/80161 – Nathan Hartley Oct 09 '15 at 14:52
  • 1
    C# 8 could simply have a null-conditional foreach of some sort, i.e. syntax like this: foreach? (var i in collection) { } I think it's a common enough scenario to justify this, and given the recent null-conditional additions to the language it makes sense here to? – mms Oct 19 '16 at 22:43

8 Answers8

174

Just as a slight cosmetic addition to Rune's suggestion, you could create your own extension method:

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

Then you can write:

foreach (var header in file.Headers.OrEmptyIfNull())
{
}

Change the name according to taste :)

Rune FS
  • 21,497
  • 7
  • 62
  • 96
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 1
    to satisfy Roslyn ```this IEnumerable source``` should be ```this IEnumerable? source``` – Pawel Sep 01 '22 at 13:08
  • @Pawel: Well that depends on the setting for nullable reference types - which didn't even exist in 2012. If you decide to go through every C# answer on Stack Overflow for 14 years and comment on every one of them that doesn't comply with an optional feature that didn't exist at the time, it's going to take you a *very* long time, with little tangible benefit. – Jon Skeet Sep 01 '22 at 13:27
  • I agree @Jon. I meant to improve (valuable) answer not to complain. – Pawel Sep 01 '22 at 13:39
  • @Pawel: My point is that there are *millions* of such answers. Anyone using nullable reference types should be aware that they haven't always existed, and be able to change their code accordingly. Note that if I *did* change it as suggested, the code would fail to compile in non-NRE-enabled contexts... and I hate the idea of every C# answer affected by this requiring two different versions of every code snippet. – Jon Skeet Sep 01 '22 at 13:41
106

Assuming that the type of elements in file.Headers is T you could do this

foreach(var header in file.Headers ?? Enumerable.Empty<T>()){
  //do stuff
}

this will create an empty enumerable of T if file.Headers is null. If the type of file is a type you own I would, however, consider changing the getter of Headers instead. null is the value of unknown so if possible instead of using null as "I know there are no elements" when null actually(/originally) should be interpreted as "I don't know if there are any elements" use an empty set to show that you know there are no elements in the set. That would also be DRY'er since you won't have to do the null check as often.

EDIT as a follow up on Jons suggestion, you could also create an extension method changing the above code to

foreach(var header in file.Headers.OrEmptyIfNull()){
  //do stuff
}

In the case where you can't change the getter, this would be my own preferred since it expresses the intention more clearly by giving the operation a name (OrEmptyIfNull)

The extension method mentioned above might make certain optimizations impossible for the optimizer to detect. Specifically, those that are related to IList using method overloading this can be eliminated

public static IList<T> OrEmptyIfNull<T>(this IList<T> source)
{
    return source ?? Array.Empty<T>();
}
Rune FS
  • 21,497
  • 7
  • 62
  • 96
  • @kjbartel's answer (at " stackoverflow.com/a/32134295/401246 " is the best solution, because it doesn't: a) involve performance degradation of (even when not `null`) degenerating the whole loop to LCD of `IEnumerable` (as using ?? would), b) require adding an Extension Method to every Project, or c) require avoiding `null` `IEnumerables` (Pffft! Puh-LEAZE! SMH.) to begin with (cuz `null` means N/A, whereas empty list means, it's applicable but is currently, well, empty!, i.e. an Employee could have Commissions that's N/A for non-Sales or empty for Sales when they haven't earned any). – Tom Apr 23 '19 at 23:59
  • @Tom aside from _one_ null check there's no penalty for the cases where the enumerator is not null. Avoiding that check while also ensuring that the enumerable is not null is impossible. The code above requires the Headers to a an `IEnumerable` which is more restrictive than the `foreach` requirements but less restrictive than the requirement of `List` in the answer you link to. Which have the same performance penalty of testing whether the enumerable is null. – Rune FS May 03 '19 at 09:36
  • I was basing the "LCD" issue on the comment by Eric Lippert on Vlad Bezden's Answer in the same thread as kjbartel's Answer: "@CodeInChaos: Ah, I see your point now. When the compiler can detect that the "foreach" is iterating over a List or an array then it can optimize the foreach to use value-type enumerators or actually generate a "for" loop. When forced to enumerate over either a list or the empty sequence it has to go back to the "lowest common denominator" codegen, which can in some cases be slower and produce more memory pressure....". Agree it requires `List`. – Tom May 06 '19 at 17:18
  • @tom The premise of the answer is that file.Headers are an IEnumerable in which case the compiler can't do the optimization. It is however rather straight forward to extend the extension method solution to avoid this. See edit – Rune FS May 07 '19 at 18:22
30

Frankly, I advise: just suck up the null test. A null test is just a brfalse or brfalse.s; everything else is going to involve much more work (tests, assignments, extra method calls, unnecessary GetEnumerator(), MoveNext(), Dispose() on the iterator, etc).

An if test is simple, obvious, and efficient.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • 1
    You do make an interesting point Marc, however, I'm currently looking to lessen the indentation levels of the code. But I will keep your comment in mind when I need to take note of performance. – Eminem Jul 31 '12 at 06:54
  • 8
    Just a quick note on this Marc.. years after I asked this question and need to implement some performance enhancements, your advice came in extremely handy. Thank you – Eminem Mar 18 '19 at 17:57
28

Using Null-conditional Operator and ForEach() which works faster than standard foreach loop.
You have to cast the collection to List though.

   listOfItems?.ForEach(item => // ... );
Andrei Karcheuski
  • 3,116
  • 3
  • 38
  • 39
18

the "if" before the iteration is fine, few of those "pretty" semantics can make your code less readable.

anyway, if the indentation disturbs your, you can change the if to check:

if(file.Headers == null)  
   return;

and you'll get to the foreach loop only when there is a true value at the headers property.

another option I can think about is using the null-coalescing operator inside your foreach loop and to completely avoid null checking. sample:

List<int> collection = new List<int>();
collection = null;
foreach (var i in collection ?? Enumerable.Empty<int>())
{
    //your code here
}

(replace the collection with your true object/type)

Tamir
  • 3,833
  • 3
  • 32
  • 41
5

The best answer in the year 2022 should be:

foreach (var h in file.Headers ?? Enumerable.Empty<T>())
{
    //do stuff
}

Replace T with your data type. if file.Headers is an array, use Array.Empty<T>() instead of Enumerable.Empty<T>()

Eddie
  • 150
  • 1
  • 11
  • Will not compile if your collection is an interface. ```Enumerable.Empty()``` or ```Array.Empty()``` is still more versatile in 2022. – Pawel Sep 01 '22 at 13:14
3

I am using a nice little extension method for these scenarios:

  public static class Extensions
  {
    public static IList<T> EnsureNotNull<T>(this IList<T> list)
    {
      return list ?? new List<T>();
    }
  }

Given that Headers is of type list, you can do following:

foreach(var h in (file.Headers.EnsureNotNull()))
{
  //do stuff
}
Wolfgang Ziegler
  • 1,675
  • 11
  • 23
  • 1
    you can use the `??` operator and shorten the return statement to `return list ?? new List;` – Rune FS Jul 31 '12 at 06:46
  • 1
    @wolfgangziegler, If I understand correctly the test for `null` in your sample `file.Headers.EnsureNotNull() != null` is not needed, and is even wrong? – Remko Jansen Jul 31 '12 at 08:56
0

For some cases I'd prefer slightly another, generic variant, assuming that, as a rule, default collection constructors return empty instances.

It would be better to name this method NewIfDefault. It can be useful not only for collections, so type constraint IEnumerable<T> is maybe redundant.

public static TCollection EmptyIfDefault<TCollection, T>(this TCollection collection)
        where TCollection: class, IEnumerable<T>, new()
    {
        return collection ?? new TCollection();
    }
N. Kudryavtsev
  • 3,556
  • 1
  • 26
  • 30