Question

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>();
}
Was it helpful?

Solution

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.

OTHER TIPS

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.

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.

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);
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top