3

I was looking at this code example in C# 7.0 and I was not sure about what was going on under the hood and the performance of this loop.

foreach (var c in text ?? throw new ArgumentNullException(nameof(text)))
{
    ...
}

My questions:

  1. Does the conditional statement get hit once or multiple times (on each iteration)?
  2. The new syntax looks different, what are the benefits to doing it this way?
Community
  • 1
  • 1
Svek
  • 12,350
  • 6
  • 38
  • 69
  • 5
    What effect did it have on performance when you tested it? You did _test_ it, didn't you? Please explain why your tests were not sufficient for you to understand the answer to your question yourself. Fix your question so that it includes a good [mcve] showing clearly exactly how you tested, and include a detailed explanation of what precisely you're still unable to understand about your tests. – Peter Duniho Feb 16 '17 at 03:18
  • 2
    Once. If `text` is null the conditional statement is hit once which should be logical without explanation. If it's looping it'll be once as well due to an enumerator is being looped through. – Quality Catalyst Feb 16 '17 at 03:26
  • @PeterDuniho I'm not sure how the question is framed incorrectly. – Svek Feb 16 '17 at 03:35
  • 3
    @Svek - Peter is suggesting that you test the code before posting, and then show us your test code, the results, etc, and then ask your question. Instead you've asked a question and expect us to do the testing. – Enigmativity Feb 16 '17 at 03:40

2 Answers2

9

In terms of "how foreach works", conditional statement will only be calculated once.

You may want to read more about how foreach loops work in these questions:
How do foreach loops work in C#?
Does foreach evaluate the array at every iteration?

Thanks to Svek for explaining that it is a new C# 7.0 feature, which will be released after Visual Studio 2017 RC:
http://structuredsight.com/2016/09/01/c-7-additions-throw-expressions/

I think that "what are benefits" is a sort of opinion-based question.
In my opinion, it brings nothing good and is just ugly in terms or code readability.
I would recommend using a widely-used common good practice:

if (text == null) // or string.IsNullOrEmpty for strings
    throw new ArgumentNullException(nameof(text));

foreach (var c in text)
{
    // ...
}

Probably, we will see null-coalescing + throw exception usage in a couple years and it will become a new standard :)

Community
  • 1
  • 1
Yeldar Kurmangaliyev
  • 33,467
  • 12
  • 59
  • 101
  • 3
    It might not be better that way. The original code may or may not be thread-safe, but your version isn't. Can you explain why you say this was is better? – Enigmativity Feb 16 '17 at 03:42
  • 3
    According to a previous paragraph, I said better in terms of "code readability and flow". Checking a value for being null in the beginning or a method and throwing `ArgumentNullException` is a **common good practice**. That's exactly what all .NET classes, 3rd-party classes and all good developers do. If everybody starts using code operators and features in the way they want, then we will stop understanding each others' code soon. Short code != better. – Yeldar Kurmangaliyev Feb 16 '17 at 03:47
  • 1
    Talking about thread safety, I assumed that `text` is a argument passed to a function - if I am right, then this code will **always be** thread-safe. It if is a class member, which can be modified from different, then there are many other techniques, approaches and functionalities which can provide thread-safety. Using null-coalescing operator for throwing new Exception is definitely not one of them. – Yeldar Kurmangaliyev Feb 16 '17 at 03:48
  • @Enigmativity I'm not actually sure if that code works at all. It shouldn't since `throw new Exception` is not of the same type as `text`, but it is required to be for a null-coalescing operator. However, if it works, then it is just ugly. – Yeldar Kurmangaliyev Feb 16 '17 at 03:51
  • 2
    @Enigmativity http://stackoverflow.com/questions/1762772/is-it-possible-to-use-operator-and-throw-new-exception – Yeldar Kurmangaliyev Feb 16 '17 at 03:52
  • 2
    @YeldarKurmangaliyev http://structuredsight.com/2016/09/01/c-7-additions-throw-expressions/ – Svek Feb 16 '17 at 03:55
  • @Svek Thanks, I didn't know about that. It looks ugly, IMO :) – Yeldar Kurmangaliyev Feb 16 '17 at 04:01
  • Please note that throw expressions were introduced to enable a very valuable and specific use case pertaining to a great feature that was going to be part of C# 7.0, namely `match` expressions, but was pulled out. This example is a consequence. A feature that did make it into the language looking for a use case. – Aluan Haddad Feb 17 '17 at 07:55
  • Also there's nothing wrong, on a type system level, with `text ?? throw`, although it is arguably imprecise (think of `Try` and `Either` types). `throw` is perfectly valid in methods that return values and the type of a `throw` expression can be conceptualized as being of a type `Never`, an uninhabited type. – Aluan Haddad Feb 17 '17 at 08:08
3

You should understand foreach inner code for understanding this C# feature. A right part of expression in foreach statement must implement IEnumerable(<T>) interface, and whole loop is, internally, a simple while, something like this:

// here can be NullReferenceException
var en = text.GetEnumerator();
while(en.MoveNext())
{
    var c = en.Current;
    {
        ...
    }
}

As you can see, there is a point in this code the NRE can occur, so you need to check the enumerable before entire loop or Enumerable extensions class, like this:

if (text.IsNullOrWhitespace())
{
    throw new ArgumentNullException(nameof(text));
}

// while loop here or text.SomeLinqCodeHere()

There are some lines of code here which aren't really unnecessary, adding some entropy without real value. In case of simple foreach it really opinion-based decision about code standards, but the real purpose of this feature is chaining it with other new things in C#7, like ?. operator, like this:

int? length = customers?.Length ?? throw new ...;
Customer first = customers?[0] ?? throw new ...;  
int? count = customers?[0]?.Orders?.Count() ?? throw new ...;

In such cases throwing the exception is similar to comment at the end of the line of code:

int? length = customers?.Length; // should not be null
Customer first = customers?[0]; // should not be null  
int? count = customers?[0]?.Orders?.Count(); // should not be null

but it adds some strict contract-like rules for your code.

As for the performance for foreach loop with such expressions, as already said, it doesn't suffer as getting the enumerator occurs only once, and before the real loop.

VMAtm
  • 27,943
  • 17
  • 79
  • 125