1

Assuming numToGenerate, min, and max are the same for both snippets and GetNextRandom is a method that uses an instance of System.Random to generate a random integer by simply returning the value of instance.Next(min, max).

First snippet using yield:

var list = new List<int>();
while(list.Count < numToGenerate)
{
    var next = GetNextRandom(min, max);
    if (!list.Contains(next))
    {
        list.Add(next);
        yield return next;
    }
}

Second snippet using normal return:

var list = new List<int>();
while(list.Count < numToGenerate)
{
    var next = GetNextRandom(min, max);
    if (!list.Contains(next))
    {
        list.Add(next);
    }
}
return list;

Let's pretend these snippets are part of a method that returns IEnumerable<int>. What are the major differences of the two? Which should I be using and why? I'm trying to understand the functional difference if any.

edhedges
  • 2,722
  • 2
  • 28
  • 61
  • so really you're asking about `yield`? Have you researched the `yield` keyword? – Jonesopolis Nov 07 '15 at 04:59
  • @Jonesopolis I have, but most of the examples I've seen don't have much logic inside the loop body. I guess I'm trying to understand what's really happening. – edhedges Nov 07 '15 at 05:02
  • 1
    Have you tried measuring the performance of each? – Enigmativity Nov 07 '15 at 05:15
  • @Enigmativity That's the next step, but I was hoping I could get someone smarter than me to explain some of the pieces I'm missing. – edhedges Nov 07 '15 at 05:17
  • Are you actually trying to use this code to return random unique values? – Enigmativity Nov 07 '15 at 05:21
  • 1
    @edhedges - Then you should use `return Enumerable.Range(0, max - min + 1).OrderBy(x => rnd.Next()).Take(numToGenerate);`. In my timing tests it's 32x faster than your code. – Enigmativity Nov 07 '15 at 05:29
  • @Enigmativity Thanks for the tip. I'll give it a shot with some more of our typical situations and see, which implementation fits our needs best. – edhedges Nov 07 '15 at 05:31

1 Answers1

1

It depends. Are you going to consume all of the values requested? If not, the first one has some advantages. For example if you call .Take(1) on it you have only been through the loop once and have only stored one value in the list.

If GetNextRandom was a very slow process and you wanted to return values to the UI as they are generated then, again, the first one has an advantage there.

BUT if you are planning on consuming all of it, of if the caller is just going to call .ToList on it to avoid enumerating it twice, then the second one is probably better, and you can adjust your return type to IList so that callers can know that they can go directly to any element and can Count the list without enumerating it again. (See also Optimize LINQ for IList)

As far as garbage collection goes, in the first one the list will be available for garbage collection after the method is complete. In the second case the caller gets the whole list and could hold it for longer.

PS Use HashSet<T> if n is large rather than inventing your own set on top of List

Community
  • 1
  • 1
Ian Mercer
  • 38,490
  • 8
  • 97
  • 133
  • I get that part, but what's happening to my `list` object? Is it just sitting there not being garbage collected until the last `yield` since it's a required part of the logic of the loop? Would it be recommended to clear out the list and/or null it out after the loop? I'm thinking it will be consumed all at once, but not entirely sure of the calling codes implementation yet (working on a collaborative project). – edhedges Nov 07 '15 at 05:07
  • 1
    Correct, it's needed until the loop ends, after that it is free and will be collected when GC happens. No need to null it, it's a local variable inside that method. I guess that is another difference that could be called out - the caller can hang on to the whole list. I'll edit. – Ian Mercer Nov 07 '15 at 05:23
  • Thanks for the `HashSet` tip! I ended up with something similar ro the first snippet, but instead am using `HashSet` and doing a `yield return nextRandom` if the `Add` method returns true. – edhedges Nov 07 '15 at 05:24