How to generate random values that don't keep giving me the same value in 'runs' of numbers?

Question

Hi I coded this OneAtRandom() extension method:

public static class GenericIListExtensions
   {
      public static T OneAtRandom<T>(this IList<T> list)
      {
         list.ThrowIfNull("list");
         if (list.Count == 0)
            throw new ArgumentException("OneAtRandom() cannot be called on 'list' with 0 elements");

         int randomIdx = new Random().Next(list.Count);
         return list[randomIdx];
      }
}

Testing it using this unit test fails:

[Test]
        public void ShouldNotAlwaysReturnTheSameValueIfOneAtRandomCalledOnListOfLengthTwo()
        {
            int SomeStatisticallyUnlikelyNumberOf = 100;
            IList<string> list = new List<string>() { FirstString, SecondString };
            bool firstStringFound = false;
            bool secondStringFound = false;

            SomeStatisticallyUnlikelyNumberOf.Times(() =>
            {
                string theString = list.OneAtRandom();
                if (theString == FirstString) firstStringFound = true;
                if (theString == SecondString) secondStringFound = true;
            });
            Assert.That(firstStringFound && secondStringFound);
        }

It seems that int randomIdx = new Random().Next(list.Count);is generating the same number 100 times in a row, I think possibly because the seed is based on the time?

How can I get this to work properly?

Thanks :)

Answer 1

You shouldn't be calling new Random()for every iteration because it causes it to be reseeded and generate the same sequence of numbers again. Create one Random object at the start of your application and pass it into your function as a parameter.

public static class GenericIListExtensions
{
      public static T OneAtRandom<T>(this IList<T> list, Random random)
      {
         list.ThrowIfNull("list");
         if (list.Count == 0)
            throw new ArgumentException("OneAtRandom() cannot be called on 'list' with 0 elements");

         int randomIdx = random.Next(list.Count);
         return list[randomIdx];
      }
}

This also has the advantage of making your code more testable as you can pass in a Random that is seeded to a value of your choice so that your tests are repeatable.

Answer 2

No; it's generating the same number 100 times because you're not seeding the generator.

Move the "new Random()" to the constructor or a static var, and use the generated object.

Answer 3

You could use a seed based on the current time to create the instance of Random. A sample on MSDN uses the following code:

int randomInstancesToCreate = 4;
Random[] randomEngines = new Random[randomInstancesToCreate];
for (int ctr = 0; ctr < randomInstancesToCreate; ctr++)
{
   randomEngines[ctr] = new Random(unchecked((int) (DateTime.Now.Ticks >> ctr)));
}