Question

I have the following function to get validation errors for a card. My question relates to dealing with GetErrors. Both methods have the same return type IEnumerable<ErrorInfo>.

private static IEnumerable<ErrorInfo> GetErrors(Card card)
{
    var errors = GetMoreErrors(card);
    foreach (var e in errors)
        yield return e;

    // further yield returns for more validation errors
}

Is it possible to return all the errors in GetMoreErrors without having to enumerate through them?

Thinking about it this is probably a stupid question, but I want to make sure I'm not going wrong.

Was it helpful?

Solution

It's definitely not a stupid question, and it's something that F# supports with yield! for a whole collection vs yield for a single item. (That can be very useful in terms of tail recursion...)

Unfortunately it's not supported in C#.

However, if you have several methods each returning an IEnumerable<ErrorInfo>, you can use Enumerable.Concat to make your code simpler:

private static IEnumerable<ErrorInfo> GetErrors(Card card)
{
    return GetMoreErrors(card).Concat(GetOtherErrors())
                              .Concat(GetValidationErrors())
                              .Concat(AnyMoreErrors())
                              .Concat(ICantBelieveHowManyErrorsYouHave());
}

There's one very important difference between the two implementations though: this one will call all of the methods immediately, even though it will only use the returned iterators one at a time. Your existing code will wait until it's looped through everything in GetMoreErrors() before it even asks about the next errors.

Usually this isn't important, but it's worth understanding what's going to happen when.

OTHER TIPS

You could set up all the error sources like this (method names borrowed from Jon Skeet's answer).

private static IEnumerable<IEnumerable<ErrorInfo>> GetErrorSources(Card card)
{
    yield return GetMoreErrors(card);
    yield return GetOtherErrors();
    yield return GetValidationErrors();
    yield return AnyMoreErrors();
    yield return ICantBelieveHowManyErrorsYouHave();
}

You can then iterate over them at the same time.

private static IEnumerable<ErrorInfo> GetErrors(Card card)
{
    foreach (var errorSource in GetErrorSources(card))
        foreach (var error in errorSource)
            yield return error;
}

Alternatively you could flatten the error sources with SelectMany.

private static IEnumerable<ErrorInfo> GetErrors(Card card)
{
    return GetErrorSources(card).SelectMany(e => e);
}

The execution of the methods in GetErrorSources will be delayed too.

I came up with a quick yield_ snippet:

yield_ snipped usage animation

Here's the snippet XML:

<?xml version="1.0" encoding="utf-8"?>
<CodeSnippets xmlns="http://schemas.microsoft.com/VisualStudio/2005/CodeSnippet">
  <CodeSnippet Format="1.0.0">
    <Header>
      <Author>John Gietzen</Author>
      <Description>yield! expansion for C#</Description>
      <Shortcut>yield_</Shortcut>
      <Title>Yield All</Title>
      <SnippetTypes>
        <SnippetType>Expansion</SnippetType>
      </SnippetTypes>
    </Header>
    <Snippet>
      <Declarations>
        <Literal Editable="true">
          <Default>items</Default>
          <ID>items</ID>
        </Literal>
        <Literal Editable="true">
          <Default>i</Default>
          <ID>i</ID>
        </Literal>
      </Declarations>
      <Code Language="CSharp"><![CDATA[foreach (var $i$ in $items$) yield return $i$$end$;]]></Code>
    </Snippet>
  </CodeSnippet>
</CodeSnippets>

I don't see anything wrong with your function, I'd say that it is doing what you want.

Think of the Yield as returning an element in the final Enumeration each time that it is invoked, so when you have it in the foreach loop like that, each time it is invoked it returns 1 element. You have the ability to put conditional statements in your foreach to filter the resultset. (simply by not yielding on your exclusion criteria)

If you add subsequent yields later in the method, it will continue to add 1 element to the enumeration, making it possible to do things like...

public IEnumerable<string> ConcatLists(params IEnumerable<string>[] lists)
{
  foreach (IEnumerable<string> list in lists)
  {
    foreach (string s in list)
    {
      yield return s;
    }
  }
}

Yes it is possible to return all errors at once. Just return a List<T> or ReadOnlyCollection<T>.

By returning an IEnumerable<T> you're returning a sequence of something. On the surface that may seem identical to returning the collection, but there are a number of difference, you should keep in mind.

Collections

  • The caller can be sure that both the collection and all the items will exist when the collection is returned. If the collection must be created per call, returning a collection is a really bad idea.
  • Most collections can be modified when returned.
  • The collection is of finite size.

Sequences

  • Can be enumerated - and that is pretty much all we can say for sure.
  • A returned sequence itself cannot be modified.
  • Each element may be created as part of running through the sequence (i.e. returning IEnumerable<T> allows for lazy evaluation, returning List<T> does not).
  • A sequence may be infinite and thus leave it to the caller to decide how many elements should be returned.

I'm surprised no one has thought to recommend a simple Extension method on IEnumerable<IEnumerable<T>> to make this code keep its deferred execution. I'm a fan of deferred execution for many reasons, one of them is that the memory footprint is small even for huge-mongous enumerables.

public static class EnumearbleExtensions
{
    public static IEnumerable<T> UnWrap<T>(this IEnumerable<IEnumerable<T>> list)
    {
        foreach(var innerList in list)
        {
            foreach(T item in innerList)
            {
                yield return item;
            }
        }
    }
}

And you could use it in your case like this

private static IEnumerable<ErrorInfo> GetErrors(Card card)
{
    return DoGetErrors(card).UnWrap();
}

private static IEnumerable<IEnumerable<ErrorInfo>> DoGetErrors(Card card)
{
    yield return GetMoreErrors(card);

    // further yield returns for more validation errors
}

Similarly, you can do away with the wrapper function around DoGetErrors and just move UnWrap to the callsite.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top