0

I'm working on a school project and I need to create an array (size of 30) of random unique numbers between 1 and 70.

I managed to find how to create an array of random unique numbers, but not between 1 and 70.

public int[] ShuffleArray(int[] arr)
{
    Random rand = new Random();
    int[] array = new int[70];
    for (int i = 0; i < 70; i++)
        array[i] = i + 1;
    int[] MyRandomArray = array.OrderBy(x => rand.Next(1, 71)).ToArray();
    for (int k = 0; k < arr.Length; k++)
        arr[k] = MyRandomArray[k];
    return arr;
}

I changed it based on feedback, but it still doesn't work.

Dan Lowe
  • 51,713
  • 20
  • 123
  • 112
Daniel
  • 41
  • 1
  • 7

1 Answers1

3

The three steps are:

  1. Generate all the choices
  2. Shuffle them
  3. Take the first 30

Looks like you got 1 and 2 covered So currently you have 70 numbers, in random order. What's left is the step 3, taking the first 30.

You can achieve it with Take. It returns an IEnumerable, so you can use ToArray to transform it into an array afterwards.

return MyRandomArray.Take(30).ToArray();

So with the code you had, it would look like this:

public int[] ShuffleArray(int[] arr)
{
    Random rand = new Random();
    int i;
    int[] array = new int[70];

    for (i = 0; i < 70; i++)
        array[i] = i + 1;

    int[] MyRandomArray = array.OrderBy(x => rand.Next()).ToArray();

    return MyRandomArray.Take(30).ToArray();
}

While we're here, let's fix a couple things too.

You can join the declaration and use of i in your for loop:

for (int i = 0; i < 70; i++)

In C#, the convention is to have variables written in camelCase, so MyRandomArray should be named myRandomArray.

You could abbreviate your for loop with the following:

int[] array = Enumerable.Range(1,70).ToArray();

Finally, if you want to really optimize things, you could join that declaration with the OrderBy and Take in a single line to drop the temporary variables:

public int[] ShuffleArray(int[] arr)
{
    Random rand = new Random();
    return Enumerable.Range(1, 70).OrderBy(x => rand.Next()).Take(30).ToArray();
}
Pierre-Luc Pineault
  • 8,993
  • 6
  • 40
  • 55
  • Thanks for the explanation, it works. can you tell me why my own code does not work? – Daniel Jan 01 '16 at 22:40
  • @Daniel in the lastest edit you did, you're missing the array declaration: `int[] arr = new int[30];`. You should also revert the `rand.Next(1, 71)` to `rand.Next()`. In your first edit, it's because you were returning all 70 numbers. – Pierre-Luc Pineault Jan 01 '16 at 22:43
  • I have the array decleration in a c# page where I call the method, but I didn't know about rand.next, thanks :) and I'm sorry if it's a common question I'm new here and they don't teach that at school. – Daniel Jan 01 '16 at 22:48
  • @Daniel No problem, just remember to be the most specific you can while asking questions and include your code, like you did in your edits. Otherwise it's pretty hard to help. – Pierre-Luc Pineault Jan 01 '16 at 22:50
  • Side note: this is valid copy-paste ready solution to homework, but if you found this post for some other reason it is wrong approach to shuffling array (proper code is O(n), while `OrderBy` solution is obviously `O(n log n)`, also proper code does not need to shuffle the rest - only 30 numbers). Make sure to search for `C# shuffle` for proper solution. – Alexei Levenkov Jan 01 '16 at 23:08
  • Indeed, an OrderBy with a `Random` or `new Guid` won't be as good as the Fisher-Yates or equivalent shuffles. It is a sub-optimal solution but will work for non-official contexts. A good implementation can be found [here](http://stackoverflow.com/questions/273313/randomize-a-listt-in-c-sharp) – Pierre-Luc Pineault Jan 01 '16 at 23:12