CODEWITHSUNDEEP
c#yield-return
Florian
Florian

Reputation: 4728

Syntax issue IEnumerable<T> method using yield return

Here is my method :

static IEnumerable<DateTime> GetMonths(DateTime from, DateTime to)
{
    // if logs is not uptodate
    TimeSpan logsMissingTimespan = to - from;

    if (logsMissingTimespan != new TimeSpan(0))
    {
        return GetMonthsBetweenTwoDates(from, to);
    }

    return null; // Why this line ?
}

private static IEnumerable<DateTime> GetMonthsBetweenTwoDates(DateTime from, DateTime to)
{

    DateTime date = from;
    DateTime lastDate = DateTime.MaxValue;

    while (date < to)
    {
        if (lastDate.Month != date.Month)
        {
            lastDate = date;
            yield return lastDate;
        }
        date = date.AddDays(1);
    }
}

it works fine but I think I can write something cleaner like this :

static IEnumerable<DateTime> GetMonths(DateTime from, DateTime to)
{
    TimeSpan logsMissingTimespan = to - from;

    if (logsMissingTimespan == new TimeSpan(0))
    {
        yield break;
    }

    return GetMonthsBetweenTwoDates(from, to);
}

But I have an error message :

Cannot return a value from an iterator. Use the yield return statement to return a value, or yield break to end the iteration.

Why should I have a return null and what is the correct syntax ?

EDIT :

So, the correct way is to use Enumerable.Empty :

static IEnumerable<DateTime> GetMonths(DateTime from, DateTime to)
{
    // if logs is not uptodate
    TimeSpan logsMissingTimespan = to - from;

    if (logsMissingTimespan != new TimeSpan(0))
    {
        return GetMonthsBetweenTwoDates(from, to);
    }

    return Enumerable.Empty<DateTime>();
}

Upvotes: 6

Views: 2199

Answers (4)

DaveShaw
DaveShaw

Reputation: 52788

Because you have used the word yield it now expects the method to yield one element at a time. It must only use yeild return or yield break to return one element per iteration.

You should use Enumerable.Empty<DateTime>(); instead of yield break.

Upvotes: 7

Turbot
Turbot

Reputation: 5227

you don't need yield break to end the iteration at your first check.

if (logsMissingTimespan == new TimeSpan(0))
{
    return null;
}    
return GetMonthsBetweenTwoDates(from, to);

Upvotes: 0

Adam Robinson
Adam Robinson

Reputation: 185593

The forms of your first two examples produce different sorts of output.

Your first example returns an IEnumerable<T> directly if the condition is satisfied, and a null reference if it is not. Your second example always returns an IEnumerable<T>, but the condition determines whether or not it has any elements.

The second example is done by using an iterator block. The yield syntax is used by the C# compiler to turn the function that you wrote into a custom (hidden) type that implements IEnumerable<T> and a type that implements IEnumerator<T>. These types implement the necessary state machines in order to achieve (hopefully) the logic that you've placed into the function. Because of this, you cannot mix paradigms; you must either return an instance of IEnumerable<T> from the function (and not use yield anywhere at all), or everything must be returned via yield.

If all you're concerned with is the fact that you're returning a null reference, you could make the methods semantically the same by returning Enumerable.Empty<DateTime> rather than null.

Upvotes: 4

Jon Skeet
Jon Skeet

Reputation: 1499770

A method is either implemented with an iterator block or it's not - so either everything is in terms of yield return and yield break, or none of it is.

However, you don't need to do anything special. Your original GetMonthsBetweenTwoDates already works where to == from, because it'll never enter the while loop.

On the other hand, your use of lastDate looks suspicious to me - in particular, it looks like it'll do something different if from happens to be in the same month as DateTime.MaxValue.

Upvotes: 3

Related Questions