Get random sets of 5 from 50

Question

From a deck of 50 need to get sets of 5 as random as possible and as fast as possible. The thought here is to shuffle to get 10 sets of five at a time with no collisions.

Int1 and Int2 will not change during a run. If they do other bad stuff would happen.

    public MainWindow()
    {
        InitializeComponent();
        Stopwatch sw = new Stopwatch();
        sw.Start();
        for(int i = 0; i < 100000; i++ )
        {
            //Debug.WriteLine(string.Join(", ", NextFive(11, 12)));
            int count = NextFive(11, 12).Count();
        }
        sw.Stop();
        Debug.WriteLine(sw.ElapsedMilliseconds.ToString("N0"));
    }
    Random rand = new Random();
    private int[] deckBase;
    int nextFiveLastStart = 45;
    private IEnumerable<int> NextFive(int int1, int int2)
    {
        if (int1 == int2)
            throw new IndexOutOfRangeException("int1 == int2");
        if (int1 > 51 || int2 > 51)
            throw new IndexOutOfRangeException("int1 > 51 || int2 > 51");
        if (deckBase == null)
        {
            deckBase = new int[50];
            int j = 0;
            for (int i = 0; i < 52; i++)
            {
                if (i == int1 || i == int2)
                    continue;
                deckBase[j] = i;
                j++;
            }
        }
        if (nextFiveLastStart >= 45)
        {
            nextFiveLastStart = 0;
            // Yates shuffle
            for (int i = 49; i >= 1; i--)
            {
                int j = rand.Next(i + 1);
                if (j != i)
                {   // exchange values
                    int curVal = deckBase[i];
                    deckBase[i] = deckBase[j];
                    deckBase[j] = curVal;
                }
            }
        }
        else
            nextFiveLastStart += 5;
        return deckBase.Skip(nextFiveLastStart).Take(5);
    }

Answer 1

 private IEnumerable<int> NextFive(int int1, int int2)

The names int1 and int2 give absolutely no clue about what they are good for. I tried to ready the code but I cannot figure it out. Looking at if (i == int1 || i == int2) I guess it means exclude but who knows.

---

throw new IndexOutOfRangeException("int1 == int2");

This isn't IndexOutOfRangeException but rather ArgumentException. The argumetns have invalid values but they might be within the allowed range.

---

throw new IndexOutOfRangeException("int1 > 51 || int2 > 51");

I agree this is the right type of the exception but I preferred it checking each value separately to give the user a hint which one of the parameters is incorrect.

---

Random rand = new Random();
private int[] deckBase;
int nextFiveLastStart = 45;

Inconsistent access modifier usage. private implicit, private explicit, private implicit... Pick one ;-)

---

int j = 0;
for (int i = 0; i < 52; i++)
{
    if (i == int1 || i == int2)
        continue;
    deckBase[j] = i;
    j++;
}

There's no need for the int j = 0; you can put it inside the for:

for (int i = 0, j = 0; i < 52; i++)
{
    if (i == int1 || i == int2)
        continue;
    deckBase[j] = i;
    j++;
}

Answer 2

1. This isn't really production code, is it? There's an obvious test method which is testing a private method. What should be the API?

2. The chunks of five are returned as

       return deckBase.Skip(nextFiveLastStart).Take(5);
   

   There are at least two problems with this line. Firstly, Skip is slow. You're losing the constant-time access of the array. Secondly, it's a lazy enumerable which keeps a reference to deckBase. If I call NextFive twenty times, hanging on to the return values each time, and then enumerate them as so:

   var seqs = Enumerable.Range(1, 10).
       Select(i => TakeFive(11, 12)).
       ToArray();
   seqs = seqs.Select(seq => seq.ToArray());
   

   then I'll only have ten distinct sequences (as compared by SequenceEqual).

I can't suggest concrete changes without rewriting the entire code, but I think it does need to be completely rewritten with a public API and eager enumerations.