PriLee
PriLee

Reputation: 43

Using Random in generating 50 random numbers in c#

I want to take 50 numbers at random so that there must be no repetition in them using random method.

Below is my code so far:

private void settext()
{      
     int i;
     Queue <int> qe= new Queue<int>(50);
     Random rm= new Random();
     for (int g = 0; g < 50; g++)
     {
         i = rm.Next(1, 50);
         if (!qe.Contains(i))
         {
              qe.Enqueue(i);
         }                
     }
 }

Upvotes: 4

Views: 2055

Answers (5)

TimothyP
TimothyP

Reputation: 21755

Following your logic it might be simpler to write this:

var results = new HashSet<int>();
var random = new Random();
while (results.Count < 20)
{
    results.Add(random.Next(1, 50));
};

There is no need to check if the number has already been added to the Hashset as every number will be added only once...

However... this is just a quick fix to help you understand the problem you're trying to solve, (it is basically the class room example) but you should really take the advice provided by Jon Skeet as you have no idea how long it will actually take to execute the code above.

Another thing to note is that you are using the Queue class which is intended to be used like a FIFO buffer... that doesn't really apply to your problem.

Upvotes: 0

Bilal Hashmi
Bilal Hashmi

Reputation: 1485

Only problem with your code is that if duplicates are found, you are still incrementing the loop and not getting all the values (50) in your queue. You may use while loop, and only increment the index when a non duplicate value is found.

int index=0;
int i;
Queue<int> qe = new Queue<int>(50);
Random rm = new Random();
while(index< 50)
{
    i = rm.Next(1, 51); //to get from 1 to 50
    if (!qe.Contains(i)) //to check for duplicate
    {
        qe.Enqueue(i);
        ++index;
    }

}

The above will generate 50 unique random numbers, if you want to take 20 numbers from them then:

var numbers = qe.Take(20);

Upvotes: 1

Jon Skeet
Jon Skeet

Reputation: 1500585

Rather than looping until you find a number which you haven't used yet, I'd suggest just creating a list (or array) with the 50 possible numbers in, and shuffling it. You can then take however many of them you want.

There are plenty of shuffling questions on Stack Overflow, such as this one.

The advantage of this is that the performance is entirely predictable and linear - whereas if you take all 50 numbers out of 50, at the end you'll have to keep generating random numbers until you happen to get the last one. Not so bad for 50, but imagine if you have hundreds of thousands of numbers...

(Also note that your existing code doesn't use the number 20 anywhere, which should ring alarm bells if you're trying to generate just 20 numbers...)

Upvotes: 6

ColinE
ColinE

Reputation: 70142

How about using Linq?

private static Random rand = new Random();

var twentyUniqueNumbers = RandomNumberStream().Distinct().Take(20);

IEnumerable<int> RandomNumberStream()
{
  yield return rand.Next(1,50);
}

Or better still, create a list of 50 numbers, shuffle then take 20 ...

var twentyUniqueNumbers = Enumerable.Range(0,50)
                                    .OrderBy(s => rand.Next());
                                    .Take(20);

This will give a more predictable performance.

Upvotes: 1

Theodoros Chatzigiannakis
Theodoros Chatzigiannakis

Reputation: 29213

You're forcing it to find 50 distinct numbers while only allowing it to generate them out of a range of 49 possible ones, I think. Try rm.Next(50) + 1 instead.

Upvotes: 1

Related Questions