# ShuffleBag (Fisher-Yates + PcgRandom)

Anything that could be done better here?

public class ShuffleBag<T> : IEnumerable<T>
{
private readonly int m_endIndex;
private readonly FastRandom m_rng;
private readonly T[] m_values;

private int m_currentIndex;

public ShuffleBag(T[] values, ulong state, ulong? stream = null) {
m_currentIndex = 0;
m_endIndex = (values.Length - 1);
m_rng = (stream.HasValue ? new FastRandom(state, stream.Value) : new FastRandom(state));
m_values = values;
}
public ShuffleBag(T[] values) : this(values, SecureRandom.GetUInt64()) { }

IEnumerator IEnumerable.GetEnumerator() {
return GetEnumerator();
}
public IEnumerator<T> GetEnumerator() {
var currentIndex = m_currentIndex;
var endIndex = m_endIndex;
var rng = m_rng;
var values = m_values;

while (currentIndex <= endIndex) {
var randomIndex = rng.NextInt32(currentIndex, endIndex);
var temp = values[randomIndex];

values[randomIndex] = values[currentIndex];
values[currentIndex] = temp;

m_currentIndex = currentIndex++;

yield return temp;
}
}
}


For those that might inquire, here's the source for FastRandom and SecureRandom.

• Quick observation - I see three different brace styles. Standardize on one. C# de-facto standard is opening and closing braces on their own lines. But that's not important - consistency is. So choose and go forth. Apr 12 '18 at 21:50
• @JesseC.Slicer I hesitate to get into the braces thing but will say that I wholeheartedly agree about consistency. The style above is enforced by the IDE automatically; have personally found that defining "classes" of braces in the reformatter helps me parse code easier/quicker (can easily be undone before checking into a repo). Apr 12 '18 at 22:44

## Enumerate once

I'm not sure whether you intended to write this to be iterable only once. Is this as designed?

[TestMethod]
public void CodeReview()
{
var bag = new ShuffleBag<string>(new[] { "a", "b", "c", "d" });

var e1 = bag.ToList();  // randomized: "b", "a", "c", "d"
var e2 = bag.ToList();  // last element: "d"
var e3 = bag.ToList();  // last element: "d"
}


## Collections interfaces

I would let the bag inherit IReadOnlyCollection<T> to provide eager access to the bag's size:

public int Count => m_values.Length;

• I did not intend for that bug but believe it has already been fixed. Jul 27 '19 at 16:55
• Ah, you are the one that uses that New static factory method. I remember from a question about native code. I've never seen that naming convention for a factory used before. Does it come from another language? Jul 27 '19 at 16:59
• It was a technique that I lifted from Java; one can pass constructors around like any other function using MyClass::new. AFAIK, C# doesn't support anything like this so I came up with static T New as an acceptable alternative. One goes a step further and always makes the actual constructors private in order to force "proper usage" upon consumers (generally just myself). Jul 27 '19 at 18:01