Any benefit of using yield in this case?

82 views Asked by At

I am maintaining some code at work and the original author is gone so thought I would ask here to see if I can satisfy my curiosity.

Below is a bit of code (anonymized) where yield is being used. As far as I can tell it does not add any benefit and just returning a list would be sufficient, maybe more readable as well (for me at least). Just wondering if I am missing something because this pattern is repeated in a couple of places in the code base.

public virtual IEnumerable<string> ValidProductTypes
{
  get
  {
    yield return ProductTypes.Type1;
    yield return ProductTypes.Type2;
    yield return ProductTypes.Type3;
  }
}

This property is used as a parameter for some class which just uses it to populate a collection:

var productManager = new ProductManager(ValidProductTypes);

public ProductManager(IEnumerable<string> validProductTypes)
{
  var myFilteredList = GetFilteredTypes(validProductTypes);
}

public ObservableCollection<ValidType> GetFilteredTypes(IEnumerable<string> validProductTypes)
{
  var filteredList = validProductTypes
                    .Where(type => TypeIsValid); //TypeIsValid returns a ValidType
  return new ObservableCollection<ValidType>(filteredList);
}
1

There are 1 answers

1
svick On

I'd say that returning an IEnumerable<T> and implementing that using yield return is the simplest option.

If you see that a method returns an IEnumerable<T>, there really is only one thing you can do with it: iterate it. Any more complicated operations on it (like using LINQ) are just encapsulated specific ways of iterating it.

If a method returns an array or list, you also gain the ability to mutate it and you might start wondering if that's an acceptable use of the API. For example, what happens if you do ValidProductTypes.Add("a new product")?

If you're talking just about the implementation, then the difference becomes much smaller. But the caller would still be able to cast the returned array or list from IEnumerable<T> to its concrete type and mutate that. The chance that anyone would actually think this was the intended use of the API is small, but with yield return, the chance is zero, because it's not possible.

Considering that I'd say the syntax has roughly the same complexity and ease of understanding, I think yield return is a reasonable choice. Though with C# 6.0 expression bodied properties, the syntax for arrays might get the upper hand:

public virtual IEnumerable<string> ValidProductTypes =>
  new[] { ProductTypes.Type1, ProductTypes.Type2, ProductTypes.Type3 };

The above answer is assuming that this is not performance-critical code, so fairly small differences in performance won't matter. If this is performance-critical code, then you should measure which option is better. And you might also want to consider getting rid of allocations (probably by caching the array in a field, or something like that), which might be the dominant factor.