Proper use of 'yield return'

I tend to use yield-return when I calculate the next item in the list (or even the next group of items).

Using your Version 2, you must have the complete list before returning. By using yield-return, you really only need to have the next item before returning.

Among other things, this helps spread the computational cost of complex calculations over a larger time-frame. For example, if the list is hooked up to a GUI and the user never goes to the last page, you never calculate the final items in the list.

Another case where yield-return is preferable is if the IEnumerable represents an infinite set. Consider the list of Prime Numbers, or an infinite list of random numbers. You can never return the full IEnumerable at once, so you use yield-return to return the list incrementally.

In your particular example, you have the full list of products, so I'd use Version 2.

The two pieces of code are really doing two different things. The first version will pull members as you need them. The second version will load all the results into memory before you start to do anything with it.

There's no right or wrong answer to this one. Which one is preferable just depends on the situation. For example, if there's a limit of time that you have to complete your query and you need to do something semi-complicated with the results, the second version could be preferable. But beware large resultsets, especially if you're running this code in 32-bit mode. I've been bitten by OutOfMemory exceptions several times when doing this method.

The key thing to keep in mind is this though: the differences are in efficiency. Thus, you should probably go with whichever one makes your code simpler and change it only after profiling.

Assuming your products LINQ class uses a similar yield for enumerating/iterating, the first version is more efficient because its only yielding one value each time its iterated over.

The second example is converting the enumerator/iterator to a list with the ToList() method. This means it manually iterates over all the items in the enumerator and then returns a flat list.

This is kinda besides the point, but since the question is tagged best-practices I'll go ahead and throw in my two cents. For this type of thing I greatly prefer to make it into a property:

public static IEnumerable<Product> AllProducts
{
    get {
        using (AdventureWorksEntities db = new AdventureWorksEntities()) {
            var products = from product in db.Product
                           select product;

            return products;
        }
    }
}

Sure, it's a little more boiler-plate, but the code that uses this will look much cleaner:

prices = Whatever.AllProducts.Select (product => product.price);

vs

prices = Whatever.GetAllProducts().Select (product => product.price);

Note: I wouldn't do this for any methods that may take a while to do their work.

And what about this?

public static IEnumerable<Product> GetAllProducts()
{
    using (AdventureWorksEntities db = new AdventureWorksEntities())
    {
        var products = from product in db.Product
                       select product;

        return products.ToList();
    }
}

I guess this is much cleaner. I do not have VS2008 at hand to check, though. In any case, if Products implements IEnumerable (as it seems to - it is used in a foreach statement), I'd return it directly.

I would have used version 2 of the code in this case. Since you have the full-list of products available and that's what expected by the "consumer" of this method call, it would be required to send the complete information back to the caller.

If caller of this method requires "one" information at a time and the consumption of the next information is on-demand basis, then it would be beneficial to use yield return which will make sure the command of execution will be returned to the caller when a unit of information is available.

Some examples where one could use yield return is:

1. Complex, step-by-step calculation where caller is waiting for data of a step at a time
2. Paging in GUI - where user might never reach to the last page and only sub-set of information is required to be disclosed on current page

To answer your questions, I would have used the version 2.

Return the list directly. Benefits:

• It's more clear
• The list is reusable. (the iterator is not) not actually true, Thanks Jon

You should use the iterator (yield) from when you think you probably won't have to iterate all the way to the end of the list, or when it has no end. For example, the client calling is going to be searching for the first product that satisfies some predicate, you might consider using the iterator, although that's a contrived example, and there are probably better ways to accomplish it. Basically, if you know in advance that the whole list will need to be calculated, just do it up front. If you think that it won't, then consider using the iterator version.

Given the exact two code snippets, I think Version 1 is the better one as it can be more efficient. Let's say there are a lot of products and the caller wants to convert to DTOs.

var dtos = GetAllProducts().Select(ConvertToDto).ToList();

With Version 2 first a list of Product objects would be created, and then another list of ProductDto objects. With Version 1 there is no list of Product objects, only the list of the required ProductDto objects gets built.

Even without converting, Version 2 has a problem in my opinion: The list is returned as IEnumerable. The caller of GetAllProducts() does not know how expensive the enumeration of the result is. And if the caller needs to iterate more than once, she will probably materialize once by using ToList() (tools like ReSharper also suggest this). Which results in an unnecessary copy of the list already created in GetAllProducts(). So if Version 2 should be used, the return type should be List and not IEnumerable.