If clause combined with yield

Question

4.89/5 (2 votes)

See more:

I do not use yield often but have run into something that I am confused by.

A code base has the following:

XML

public IEnumerable<dynamic> Points
{
    get {
        foreach (var o in GetData) yield return o;
    }
}

private IEnumerable<dynamic> GetData()
{
    if (Model.Plan == null ||
        Model.Plan.Points == null ||
        Model.Plan.Points.Datas== null)
    {
        yield return null;
    }

    foreach (var data in Model.Plan.Points.Datas)
    {
          yield return data;
    }
}

Please don't get hung up on the objects and their names. It really shouldn't matter.

The behavior I am seeing is the Model.Plan.Points is null, but I get a null reference in the foreach loop that is referencing the datas inside the Points. Shouldn't the previous conditional not allow that?
if(Model.Plan.Points == null) yield return null;

Posted 12-Feb-14 4:55am

N_tro_P

Add a Solution

3 solutions

Solution 3

While yield return o does return null here, the question is what are you doing with it afterwards? Basically, the yield return null is sending null back up to the Points call and is returned back to the level above. What you need to do in this call is wrap the yield return o up so that it doesn't yield if it gets null. Rewrite it like this:

C#

public IEnumerable<dynamic> Points
{
  get
  {
    foreach (var o in GetData())
    {
      if (o != null)
      {
        yield return o;
      }
    }
  }
}

Now you can call this confidently knowing that you will get data without null values outside.

Okay, I see the issue you are getting now. The point that's causing you a problem is a misunderstanding of what yield actually does here. Basically, while it yields a value back to the caller, it will still continue processing inside this method. It doesn't act like a return statement, it merely yields the value back to the caller. Potentially, with a yield statement, you could have multiple values coming out of one call.

So, the fact that the next statement works on Model.Plan.Points may fail because Model.Plan or Model.Plan.Points is null. You could work around this by putting that part in an else clause.

[Edit]Here's a way to really blow your mind with a yield - assume that the model is fully populated :

C#

private IEnumerable<dynamic> GetData()
{
  foreach (var data in Model.Plan.Points)
  {
    yield return data;
  }
  foreach (var data in Model.Plan.Points)
  {
    yield return data;
  }
}

Posted 12-Feb-14 6:12am

Pete O'Hanlon

Updated 12-Feb-14 6:49am

v3

Comments

[no name] 12-Feb-14 12:25pm

That does not seem to solve the problem. I added the conditional but I still get a null reference on the foreach.
Resharper is telling me I have a potential null reference as well. I am confused as to why. It seems it should be caught in it. I am able to get it to run safely by placing the foreach inside of the inverted if statement. But it seems that should not be necessary.

Pete O'Hanlon 12-Feb-14 12:50pm

I updated my answer to explain what's really going on - basically yield doesn't behave in the way you are expecting it to.

[no name] 12-Feb-14 14:04pm

OK, thank you. That makes sense now :-)
+5

Pete O'Hanlon 12-Feb-14 14:18pm

You're welcome. I'm glad to be able to help.

Solution 2

I think yield return does not exit from the method.
Maybe:

C#

    if (Model.Plan == null ||
        Model.Plan.Points == null ||
        Model.Plan.Points.Datas== null)
    {
       return null;
    }
// ...

Posted 12-Feb-14 5:56am

phil.o

Comments

[no name] 12-Feb-14 12:00pm

That will not compile as an iterator (a method with a yield) can not contain 'return'. It must yield return.

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Matt T Heffron · Accepted Answer · 2014-02-12T06:52:00

Because the yield return null; leaves the iterator in a state where it believes there are more items, the foreach loop in Points next goes to the foreach loop here, which then gives the null reference exception.
So this method must not proceed to the foreach if the null-check branch is taken.
Just put the foreach in the else clause!

C#

private IEnumerable<dynamic> GetData()
{
    if (Model.Plan == null ||
        Model.Plan.Points == null ||
        Model.Plan.Points.Datas== null)
    {
        yield return null;
    }
    else
    {
        foreach (var data in Model.Plan.Points.Datas)
        {
            yield return data;
        }
    }
}

That being said, this can be greatly simplified, since Model.Plan.Points.Datas is obviously an IEnumerable<dynamic>:

C#

private IEnumerable<dynamic> GetData()
{
    if (Model.Plan == null ||
        Model.Plan.Points == null ||
        Model.Plan.Points.Datas== null)
    {
        return Enumerable.Empty<dynamic>();
    }
    else
    {
        return Model.Plan.Points.Datas;  //.Cast<dynamic>() if necessary.
    }
}