1

So basically, I need to have the numbers 1-10 randomly ordered upon startup into an array, and on when the form loads, it should load the first number. Each time the user clicks a button, it will load the info associated with the next number. This is my code, but for some reason it generates a number that is not an integer a lot.

Random rng = new Random(10);
int[] QuestionOrder = new int[10];

for (int i = 0; i < QuestionOrder.Length; i++)
{
    int temp = rng.Next(1,10);
    while(!(QuestionOrder.Contains(temp)))
    {
        QuestionOrder[i] = temp;
    }
}

each time it generates a number 1 - 10 and checks if it has already been stored in the array, if not, stores it and runs again.

For some reason, its generating numbers that are not integers 1 - 10, and i cant figure out why.

Ondrej Janacek
  • 12,486
  • 14
  • 59
  • 93
  • What _are_ the numbers you get? Have you tried putting some breakpoints and stepping through the code to see what happens? Please try to debug your code yourself first, then ask (or rather search) for solutions to problems you encounter. – CodeCaster Mar 22 '14 at 13:39
  • Also read about hashset – Gilad Mar 22 '14 at 13:43
  • Welcome to Stack Overflow! Please, do not include information about a language used in a question title unless it wouldn't make sense without it. Tags serve this purpose. – Ondrej Janacek Mar 22 '14 at 14:14

6 Answers6

1

I think you need to overwrite the value of temp inside the while loop and change your range to 1-11 instead of 1-10 and use an if statement before the loop:

int temp = rng.Next(1, 11);
if(!QuestionOrder.Contains(temp)) QuestionOrder[i] = temp;
else 
{
    while(QuestionOrder.Contains(temp))
    {
       temp = rng.Next(1, 11);
       QuestionOrder[i] = temp;
    }
}
Selman Genç
  • 100,147
  • 13
  • 119
  • 184
1

You could generate ten random numbers first, then iterate through them.

var rnd = new Random(Environment.TickCount);
var arr = Enumerable.Range(1,10).OrderBy(x => rnd.Next()).ToArray();
Ismail Hawayel
  • 2,167
  • 18
  • 16
0

First, your seed is always the same. Either omit it, or use maybe the current date. Second, you're generating numbers between 1 and 9, so you will never get 10, and one of your array cell will stay empty. Change temp = rng.Next(1, 11);

Saverio Terracciano
  • 3,885
  • 1
  • 30
  • 42
0

The best algorithm to shuffle an array is Knuth-Fisher-Yates, that can be implemented as follows:

public static class Extensions
{
    public static void Shuffle<T>(this IList<T> list, Random rand)
    {
       for (int i = list.Count - 1; i > 0; i--)
       {
           int n = rand.Next(i + 1);
           int tmp = list[i];
           list[i] = list[n];
           list[n] = tmp;
       }
    }
}

Usage:

// to get different random sequences every time, just remove the seed (1234)
Random rng = new Random(1234); 
// create an array containing 1,2...9,10
int[] questionOrder = Enumerable.Range(1,10).ToArray();
// shuffle the array
questionOrder.Shuffle(rand);

Side note:

To know why Knuth-Fisher-Yates algorithm is better than a naive implementation, read this interesting post by Jeff

digEmAll
  • 56,430
  • 9
  • 115
  • 140
0

As others have pointed out, you're not regenerating a new number when the generated number is already present in the list, leaving you with various default values of 0 in your result.

There are however better ways to 'shuffle' a list of numbers, see Randomize a List in C#:

public static IList<T> 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;
    }

    return list;
}

Which you can call like this:

IList<int> QuestionOrder = Enumerable.Range(1, 10)
                                     .ToList()
                                     .Shuffle();
Community
  • 1
  • 1
CodeCaster
  • 147,647
  • 23
  • 218
  • 272
0

this does work nicely

    static void Main(string[] args)
    {
        Random random = new Random();

        var range = Enumerable.Range(1, 10).ToList();
        int[] rnd = new int[10];

        int j = 0;
        do
        {
            int i = random.Next(0, range.Count);
            rnd[j++] = range[i];
            range.RemoveAt(i);
        }
        while(range.Count > 1);

        rnd[j] = range[0];
    }

less cpu intensive than using a contains in a loop

Fredou
  • 19,848
  • 10
  • 58
  • 113