How can I get true randomness in this class without Thread.Sleep(300)?

Question

I've made a class (code below) that handles the creation of a "matching" quiz item on a test, this is the output:

It works fine.

However, in order to get it completely random, I have to put the thread to sleep for at least 300 counts between the random shuffling of the two columns, anything lower than 300 returns both columns sorted in the same order, as if it is using the same seed for randomness:

LeftDisplayIndexes.Shuffle();
Thread.Sleep(300);
RightDisplayIndexes.Shuffle();

What do I have to do to make the shuffling of the two columns completely random without this time wait?

full code:

using System.Collections.Generic;
using System;
using System.Threading;

namespace TestSort727272
{
    class Program
    {
        static void Main(string[] args)
        {
            MatchingItems matchingItems = new MatchingItems();
            matchingItems.Add("one", "111");
            matchingItems.Add("two", "222");
            matchingItems.Add("three", "333");
            matchingItems.Add("four", "444");
            matchingItems.Setup();

            matchingItems.DisplayTest();
            matchingItems.DisplayAnswers();

            Console.ReadLine();

        }
    }

    public class MatchingItems
    {
        public List<MatchingItem> Collection { get; set; }
        public List<int> LeftDisplayIndexes { get; set; }
        public List<int> RightDisplayIndexes { get; set; }

        private char[] _numbers = { '1', '2', '3', '4', '5', '6', '7', '8' };
        private char[] _letters = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h' };

        public MatchingItems()
        {
            Collection = new List<MatchingItem>();
            LeftDisplayIndexes = new List<int>();
            RightDisplayIndexes = new List<int>();
        }

        public void Add(string leftText, string rightText)
        {
            MatchingItem matchingItem = new MatchingItem(leftText, rightText);
            Collection.Add(matchingItem);
            LeftDisplayIndexes.Add(Collection.Count - 1);
            RightDisplayIndexes.Add(Collection.Count - 1);
        }

        public void DisplayTest()
        {
            Console.WriteLine("");
            Console.WriteLine("--TEST:-------------------------");
            for (int i = 0; i < Collection.Count; i++)
            {
                int leftIndex = LeftDisplayIndexes[i];
                int rightIndex = RightDisplayIndexes[i];
                Console.WriteLine("{0}. {1,-12}{2}. {3}", _numbers[i], Collection[leftIndex].LeftText, _letters[i], Collection[rightIndex].RightText);
            }
        }
        public void DisplayAnswers()
        {
            Console.WriteLine("");
            Console.WriteLine("--ANSWERS:-------------------------");
            for (int i = 0; i < Collection.Count; i++)
            {
                string leftLabel = _numbers[i].ToString();
                int leftIndex = LeftDisplayIndexes[i];
                int rightIndex = RightDisplayIndexes.IndexOf(leftIndex);
                string answerLabel = _letters[rightIndex].ToString();

                Console.WriteLine("{0}. {1}", leftLabel, answerLabel);

            }
        }

        public void Setup()
        {
            do
            {
                LeftDisplayIndexes.Shuffle();
                Thread.Sleep(300);
                RightDisplayIndexes.Shuffle();
            } while (SomeLinesAreMatched());
        }

        private bool SomeLinesAreMatched()
        {
            for (int i = 0; i < LeftDisplayIndexes.Count; i++)
            {
                int leftIndex = LeftDisplayIndexes[i];
                int rightIndex = RightDisplayIndexes[i];
                if (leftIndex == rightIndex)
                    return true;
            }
            return false;
        }


        public void DisplayAsAnswer(int numberedIndex)
        {
            Console.WriteLine("");
            Console.WriteLine("--ANSWER TO {0}:-------------------------", _numbers[numberedIndex]);
            for (int i = 0; i < Collection.Count; i++)
            {
                int leftIndex = LeftDisplayIndexes[i];
                int rightIndex = RightDisplayIndexes[i];

                Console.WriteLine("{0}. {1,-12}{2}. {3}", _numbers[i], Collection[leftIndex].LeftText, _letters[i], Collection[rightIndex].RightText);
            }
        }
    }

    public class MatchingItem
    {
        public string LeftText { get; set; }
        public string RightText { get; set; }

        public MatchingItem(string leftText, string rightText)
        {
            LeftText = leftText;
            RightText = rightText;
        }
    }

    public static class Helpers
    {
        public static void Shuffle<T>(this IList<T> list)
        {
            Random rng = new Random();
            int n = list.Count;
            while (n > 1)
            {
                n--;
                int k = rng.Next(n + 1);
                T value = list[k];
                list[k] = list[n];
                list[n] = value;
            }
        }
    }

}

Answer 1

Move Random rng = new Random(); to a static variable.

MSDN says "The default seed value is derived from the system clock and has finite resolution". When you create many Random objects within a small time range they all get the same seed and the first value will be equal to all Random objects.

By reusing the same Random object you will advance to the next random value from a given seed.

Answer 2

The problem is that you are creating your Random objects too close to each other in time. When you do that, their internal pseudo-random generators are seeded with the same system time, and the sequence of numbers they produce will be identical.

The simplest solution is to reuse a single Random object, either by passing it as an argument to your shuffle algorithm or storing it as a member-variable of the class in which the shuffle is implemented.

Answer 3

Only make one instance of the Random class. When you call it without a constructor it grabs a random seed from the computer clock, so you could get the same one twice.

public static class Helpers
{
    static Random rng = new Random();
    public static void Shuffle<T>(this IList<T> list)
    {
        int n = list.Count;
        while (n > 1)
        {
            n--;
            int k = rng.Next(n + 1);
            T value = list[k];
            list[k] = list[n];
            list[n] = value;
        }
    }
}

Answer 4

I have to put the thread to sleep for at least 300 counts between the random shuffling of the two columns, anything lower than 300 returns both columns sorted in the same order, as if it is using the same seed for randomness

You've answered your own question here. It is "as if it is using the same seed" because it is using the same seed! Due to the relatively coarse granularity of the Windows system clock, multiple Random instances constructed at nearly the same time will have the same seed value.

As Albin suggests, you should just have one Random object and use that. This way instead of a bunch of pseudorandom sequences that all start at the same seed and are therefore identical, your Shuffle method will be based on a single pseudorandom sequence.

Considering that you have it as an extension method, you may desire for it to be reusable. In this case, consider having an overload that accepts a Random and one that doesn't:

static void Shuffle<T>(this IList<T> list, Random random)
{
    // Your code goes here.
}

static void Shuffle<T>(this IList<T> list)
{
    list.Shuffle(new Random());
}

This allows the caller to provide a static Random object if he/she's going to be calling Shuffle many times consecutively; on the other hand, if it's just a one-time thing, Shuffle can take care of the Random instantiation itself.

One last thing I want to point out is that since the solution involves using a single shared Random object, you should be aware that the Random class is not thread-safe. If there's a chance you might be calling Shuffle from multiple threads concurrently, you'll need to lock your Next call (or: what I prefer to do is have a [ThreadStatic] Random object for each thread, each one seeded on a random value provided by a "core" Random -- but that's a bit more involved).

Otherwise you could end up with Next suddenly just retuning an endless sequence of zeroes.

Answer 5

The way random generators work, roughly, is that they have a seed from which the random values are derived. When you create a new Random object, this seed is set to be the current system time, in seconds or milliseconds.

Let's say when you create the first Random object, the seed is 10000. After calling it three times, the seeds were 20000, 40000, 80000, generating whatever numbers form the seeds (let's say 5, 6, 2). If you create a new Random object very quickly, the same seed will be used, 10000. So, if you call it three times, you'll get the same seeds, 20000, 40000, and 80000, and the same numbers from them.

However, if you re-use the same object, the latest seed was 80000, so instead you'll generate three new seeds, 160000, 320000, and 640000, which are very likely to give you new values.

That's why you have to use one random generator, without creating a new one every time.

Answer 6

Try to use Random() just one time. You'll get the idea.