6

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>();
}
2

4 Answers 4

7

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.

4

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.

3

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.

6
  • The example is stupid, it's just for illustrate my purpose :-)
    – Florian
    Nov 29, 2011 at 13:22
  • @Florian: Then I suggest you pick a better example in future :) Seriously, it helps if it's representative of what you're actually trying to do, as there may well be a better approach.
    – Jon Skeet
    Nov 29, 2011 at 13:40
  • Ok, I take your notice for the future.
    – Florian
    Nov 29, 2011 at 13:54
  • Even if I think I'll never use stackoverflow to ask questions because I'll receive a very good book[1] very soon :-) [1]manning.com/skeet2
    – Florian
    Nov 29, 2011 at 13:58
  • @Florian: What Jon doesn't mention is that his books are just the output of a sophisticated AI that scrapes answers from SO ;) Nov 29, 2011 at 14:13
0

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);

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.