c# how to prevent duplicate numbers populating from a random number algorithm?

Question

I am building a c# windows application form. I have so far produced a random number algorithm which populates a list box with 30 (int) random numbers. I am not using an array as the list box is an array itself.

My problem is when I click on a button to generate 30 random numbers. the numbers are populated but I am then at risk of producing duplicate numbers which I want to prevent from happening.

I would like to be able to use a HashSet or Distinct method to prevent duplication. I have used a simple .Distinct to remove duplicates but it doesn't prevent duplicates from being populated to the list.

note: I can't sort the list box as I have separate functionality for that.

private void GenerateNumbers_Click(object sender, EventArgs e)
        {
            List<int> nums = new List<int>();
            Random rnd = new Random();
            //HashSet<int> duplicate = new HashSet<int>();
            //IEnumerable<int> RemoveDuplicates = numberList.Distinct();
            int produce;

            for (int i = 0; i < 30; i++)
            {
                produce = rnd.Next(1, 250);
                this.nums.Items.Insert(0, produce.ToString());
            }

            GenerateNumbers.Enabled = false; 
        }

what is `GenerateNumbers.Enabled = false` do.. does this call the method again.. if so this is the problem read the following link for hints https://stackoverflow.com/questions/14673876/multiple-random-numbers-are-the-same — MethodMan, Nov 30 '17 at 21:47
Why not change your loop condition? Loop until you get 30 distinct integers. — Dido, Nov 30 '17 at 21:48
Random numbers by definition are allowed to create duplicates. If you do not want duplicates you want a shuffle, not a random. — Dour High Arch, Nov 30 '17 at 21:50
GenerateNumbers.Enabled = false; this simply disables the button after it's been clicked. it doesn't cause any problems. — Tom, Nov 30 '17 at 21:52
@Dour High Arch I have a shuffle method for that just to randomise the numbers afterward. I would like the random numbers not be duplicates. — Tom, Nov 30 '17 at 21:56
@Dido how would I go about changing the condition I couldn't simply do for(int i = 0; i < 30.Distinct; i ++)? — Tom, Nov 30 '17 at 21:58

score 2 · Answer 1 · answered Nov 30 '17 at 21:55

So you want 30 numbers from 1 to 250? Create an array with all the numbers, shuffle it, and then take the first 30. For example:

Random rand = new Random();

// create an array with 250 numbers, in order
var numbers = Enumerable.Range(1, 250).Select(x => x).ToArray();
// shuffle
for (int i = numbers.Length - 1; i > 0; --i)
{
    int n = rand.Next(i+1);
    int t = numbers[i];
    numbers[i] = numbers[n];
    numbers[n] = t;
}

// from here, just take the first 30 numbers

Patrick Artner · Answer 2 · 2017-11-30T22:12:00.573

I would go with an Enumerator:

    // will produce as many ints as needed (with duplicates).
    // do not call: YieldRandomInt(1,250).ToList();
    static IEnumerable<int> YieldRandomInt(int min, int max)
    {
        Random r = new Random();

        while (true)
            yield return r.Next(min, max);
    }

    // Gets as many distinct ints as needed and produces a list for you
    static IList<int> GetDistinctRandomInts(int count, int min, int max)
    {
        return YieldRandomInt(min, max).Distinct().Take(count).ToList();
    }

Your algo would simply call var rnds = GetDistinctRandomInts(30,1,250);

The shuffle approach is nice as well, shuffling for poor(or rich?) ones:

Enumerable.Range(1, 250).OrderBy(r => Guid.NewGuid()).Take(30).ToList();

This will create 250 Guids on each call and re-sort the created range - wich takes some effort. Its Randomness is not computationally/ cryptografically sound - but Random is neither. (Still: if you call that multiple times, go with Random )

Unfortunately, I am unable to use the following example. To generate the list it needs to be within the GenerateNumbers_Click method. — Tom, Nov 30 '17 at 22:19
@Tom that sounds like a arbitary restraint that should be mentioned in the questions - which puts it kindof into Homework-Land. Normally you should strife for clean code and reusability - hence smaller methods. But youve tgot multiple answers to choose from, so one might help you out. — Patrick Artner, Nov 30 '17 at 22:23

score 1 · Answer 3 · edited Nov 30 '17 at 22:19

1

Change your algorithm a bit:

 private void GenerateNumbers_Click(object sender, EventArgs e)
        {
            List<int> nums = new List<int>(30);
            Random rnd = new Random();
            while (true)
            {
                int num = rnd.Next(1, 250);
                if (!nums.Contains(num))
                {
                    nums.Add(num);
                }
                if (nums.Count == 30)
                {
                    break;
                }                
            }
            // elided
        }

Proposal Edit:

    private void GenerateNumbers_Click(object sender, EventArgs e)
    {
        var nums = new HashSet<int>();
        Random rnd = new Random();

        while (nums.Count < 30)
            nums.Add(rnd.Next(1, 250));

        // use nums.ToList();
    }

edited Nov 30 '17 at 22:19

Patrick Artner

50,409
9
43
69

answered Nov 30 '17 at 21:57

Dido

520
2
7
21

If you've used a Hashset and populated it until its Count == 30 you could avoid some costly Contains-Checks and shorten it a bit – Patrick Artner Nov 30 '17 at 22:08
You're right, it can be optimised. `Hashset`'s `Contains` method should be O(1). That being said, I doubt it will have a measurable performance difference. – Dido Nov 30 '17 at 22:15
Sure, go ahead. – Dido Nov 30 '17 at 22:17

c# how to prevent duplicate numbers populating from a random number algorithm?

3 Answers3