Will this always generate UNIQUE RANDOM number each time?

Question

Below i have written a code which is working fine and Generating a Random number its working logic is that before to return Random Number it compares random number with already generated numbers in textfile. I want to assure that will it produce unique Random number each time?

    public int GenerateRandomNo()
    {
        int _min = 0000;
        int _max = 9999;
        Random _rdm = new Random();
        return _rdm.Next(_min, _max);
    }
    public int rand_num()
    {

        string file_path = System.IO.Path.GetDirectoryName(System.Windows.Forms.Application.ExecutablePath) + @"\Invoices\numbers.txt";
        int number = File.ReadLines(file_path).Count(); //count number of lines in file
        System.IO.StreamReader file = new System.IO.StreamReader(file_path);
        if (number == 0)
        {
            randnum = GenerateRandomNo();

        }
        else
        {
            randnum = GenerateRandomNo();
            for (int a = 1; a <= number; a++)
            {
                if ((file.ReadLine()) == randnum.ToString())
                    randnum = GenerateRandomNo();
            }
            file.Close();
        }
        createText = randnum.ToString() + Environment.NewLine;
        File.AppendAllText(file_path, createText);
        file.Close();
        return randnum;

    }

If their is any improvement needed in code, then please tell me. As i want to completely assure that it will always Generate Unique Random Number.

Yes i want to get a unique number always when program is run again. — yaseen enterprises, Jul 19 '18 at 03:14
In that case you likely want a database with a PK or unique index. Or use something more appropriate, like a GUID (which is not guaranteed unique, but the probability of a collision is much lower). — mjwills, Jul 19 '18 at 03:14
Ok i got your point, Use of database is a more optimum solution. One more question is that please have a look on working logic of my code and tell me is there any loop hole in it which can lead it to not generate Unique Random Number. — yaseen enterprises, Jul 19 '18 at 03:15
Random does not mean unique. If you just need to use the numbers 0-9999 once, use Enumberable.Range and shuffle them. Your code is also flawed in that it creates a new Random object each time - call it in a loop and it will almost certainly create dupes — Ňɏssa Pøngjǣrdenlarp, Jul 19 '18 at 03:18
This feels like a XY Problem - https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem . **Why** do you want to do this? — mjwills, Jul 19 '18 at 03:19
Thats not what XY means - you havent actually described the problem being solved, just the "solution" you are wrestling with. — Ňɏssa Pøngjǣrdenlarp, Jul 19 '18 at 03:24
@yaseenenterprises - Just a small thing too - calling `_rdm.Next(0, 9999)` will only produce numbers between 0 and 9998 inclusively. The second parameter is an exclusive upper bound. — Enigmativity, Jul 19 '18 at 03:49

score 2 · Accepted Answer · answered Jul 19 '18 at 03:45

Here is a very efficient way to do this task:

private Random rnd = new Random();

public int rand_num()
{
    string exe_path = System.Windows.Forms.Application.ExecutablePath;
    string exe_folder = System.IO.Path.GetDirectoryName(exe_path);
    string file_path = System.IO.Path.Combine(exe_folder, "Invoices\numbers.txt");

    var number =
        Enumerable
            .Range(0, 10000)
            .Except(File.ReadAllLines(file_path).Select(x => int.Parse(x)))
            .OrderBy(x => rnd.Next())
            .First();

    File.AppendAllLines(file_path, new [] { number.ToString() });

    return number;
}

It generates a list of all of the numbers Enumerable.Range(0, 10000). It then removes the numbers that it already finds in the text file .Except(File.ReadAllLines(file_path).Select(x => int.Parse(x))). It then orders the remaining numbers randomly (like shuffling a pack of cards) .OrderBy(x => rnd.Next()) and finally it just selects the first number .First();.

Doesn't seem like it would be efficient to read the file every time you need a value. — David Heffernan, Jul 19 '18 at 05:18
@DavidHeffernan - It would be perfectly fine to do so if you need to get a different random number between invocations of the program, i.e. running it once per day and needing a new number each day. — Enigmativity, Jul 20 '18 at 00:06

Flydog57 · Answer 2 · 2018-07-19T15:21:36.110

Here's a way to get unique random numbers. It uses a HashSet as was suggested by another contributor. Note that there is a single, instance-level instance of System.Random (you shouldn't be creating new ones all the time), and it's seeded with an indication of the time, which will result in a new sequence of random numbers each time.

    private readonly System.Random _random = new System.Random((int) (DateTime.UtcNow.Ticks & 0x7fff_ffff));
    private readonly HashSet<int> _previousNumbers = new HashSet<int>();
    private const int Min = 0;
    private const int Max = 9999;
    public int GetNextRandom()
    {
        int next;
        do
        {
            next = _random.Next(Min, Max);
        } while (_previousNumbers.Contains(next) && _previousNumbers.Count < Max-Min);

        _previousNumbers.Add(next);
        return next;
    }

Also note that it will stop handing out random number once there are none left to hand out (since they need to be unique, there can only be Max-Min calls of this before the well is exhausted). It will also get slower as that max number of calls is reached (since the likelihood of collisions keeps going up).

I haven't tried to decipher your code as posted, it's a bit muddled (and, it won't compile if just copied/pasted). But, your question was about generating unique random numbers.

However, there are a couple of points worth mentioning:

1. What are you trying to do

The comment from @mjwills (above) is the most pertinent one. Why do you want to do this?

2. Skipping previously generated random numbers likely reduces randomness

Pseudorandom number generators like System.Random create a sequence of numbers that show no autocorrelation at any frequency. It's the sequence that's random, not any particular number. By skipping some of them (the duplicates) you may be breaking that randomness.

3. System.Random is often the wrong choice

System.Random is not a good random number generator for most applications. System.Random is a repeatable pseudorandom number generator. If you give it the same seed, it will give you exactly the same sequence pseudorandom numbers over and over again (that's why I seed it from the clock above). If you really want to use this kind of algorithm, and you don't want this behavior, you need to get a seeding mechanism that has a lot of entropy (like a good random number generator, hey, oops!).

If you want to create a simple game - say a "Magic 8-Ball", System.Random (seeded with machine ticks like above) might be the right tool. If you want to have a game that includes wagering, don't do this (and, if you want to read about how not to use System.Random, look at the Wikipedia article on the casino in Montreal (search for "keno")).

If you want a better repeatable pseudorandom number algorithm, consider a Mersenne Twister implementation.

If you want to do something like create a gambling machine or do cryptography, you need something like System.Security.Cryptography.RNGCryptoServiceProvider. Generate a sequence of 128-bit random numbers and your are pretty much guaranteed unique-ness (or, for that matter, just use GUIDs).

Generating random numbers isn't the problem for the OP. He wants to generate a unique random number. The quality of the random numbers isn't what this question is about. But what you've written is correct though, but it just doesn't answer this question. — Enigmativity, Jul 19 '18 at 04:40
Thanks, but, geez, two downvotes in about 5 minutes (someone else anonymously dinged another of my answers). I need to go to bed. Note that I finished up by saying "use a HashSet as is described in one of the comments". For what it's worth, eliminating duplicates likely makes the output of System.Random less random (remember, a pseudorandom number algorithm's randomness has to do with the fact that the sequence has no autocorrelation at any frequency). Removing dups will change that sequence and perhaps induce some auto-correlation. The big question is why he wants to do this. — Flydog57, Jul 19 '18 at 04:43

Josh Withee · Answer 3 · 2018-07-19T16:42:27.440

My original answer didn't really answer the real problem so I'm updating my answer to see if I can add something of value.

Here is an alternative version of Enigmativity's answer which allows you to use a file that separates numbers with any characters, instead of just line breaks:

private Random rnd = new Random();

public int rand_num()
{
    string exe_path = System.Windows.Forms.Application.ExecutablePath;
    string exe_folder = System.IO.Path.GetDirectoryName(exe_path);
    string file_path = System.IO.Path.Combine(exe_folder, "Invoices\numbers.txt");

    string[] commaSeparatedValues = Regex.Split(System.IO.File.ReadAllText(file_path), "[^0-9]+");
    IEnumerable<int> usedValues = commaSeparatedValues.Select(x => int.Parse(x)).AsEnumerable();
    int[] availableValues = Enumerable.Range(0, 9999).Except(usedValues).ToArray();
    int number = availableValues[rnd.Next(0, availableValues.Count())];

    File.WriteAllText(file_path, commaSeparatedValues + "," + number);

    return number;
}

Original answer:

This is not a good way to create a method for generating random numbers. Here's the problem:

Every time you call GenerateRandomNo(), it creates a new Random object, which is seeded using the current time. Now suppose you have a loop in which you want to generate random numbers [which you do!], and you use that method. For example:

for(int i=0; i<10; i++){
    Console.WriteLine("Here is a number: " + GenerateRandomNo());
}

This will happen so fast that you may be instantiating Random objects with the same seed! Those separate instantiations will give you the same number when you ask for a random number, since they have the same seed.

To solve this problem, you want your code to instantiate one instance of Random and utilize it as many times as you need it. For example:

public int GenerateRandomNo(Random r)
{
    int _min = 0000;
    int _max = 9999;
    return r.Next(_min, _max);
}
public int rand_num()
{
    Random rdm = new Random();
    string file_path = System.IO.Path.GetDirectoryName(System.Windows.Forms.Application.ExecutablePath) + @"\Invoices\numbers.txt";
    int number = File.ReadLines(file_path).Count(); //count number of lines in file
    System.IO.StreamReader file = new System.IO.StreamReader(file_path);
    if (number == 0)
    {
        randnum = GenerateRandomNo(rdm);
    }
    else
    {
        randnum = GenerateRandomNo(rdm);
        for (int a = 1; a <= number; a++)
        {
            if ((file.ReadLine()) == randnum.ToString())
                randnum = GenerateRandomNo(rdm);
        }
        file.Close();
    }
    createText = randnum.ToString() + Environment.NewLine;
    File.AppendAllText(file_path, createText);
    file.Close();
    return randnum;
}

The issue with the seed isn't the problem with the OP's code. It might be a problem if he gets the reading of the file right, but it's not causing the code to fail now. — Enigmativity, Jul 19 '18 at 03:51
Up to you. Leave the answer as is. Change the answer to be more on point. Or delete it. Your choice. — Enigmativity, Jul 19 '18 at 10:37

score -2 · Answer 4 · answered Jul 19 '18 at 03:32

I suggest you use DateTime.UtcNow.Ticks.ToString()*.

I believe it will work, up to a point where it will catastrophically fail. In other words the code will work correctly until it doesn't work at all (file is too big).

I suggest one improvement: Store the initial seed in a file.

* It is possible for DateTime.UtcNow.Ticks.ToString() to return the same value if you configure computer to go back in time, but that's similar to making edits to the file you use in your solution.

There code shows max of 9999, but he requirement doesn't specify this requirement. — tymtam, Jul 19 '18 at 03:48

score -4 · Answer 5 · edited Jul 19 '18 at 14:52

-4

No, it doesn't work, use this case:

Your file:

Your procedure:

- generate the first random number = 4
- found the random number in the file
- generate antoher random number = 4 (yes, still 4)
- fail!

I tried to fix your code without running it.

private Random _rdm = new Random();

public int GenerateRandomNo()
{
    int _min = 0000;
    int _max = 9999;
    return _rdm.Next(_min, _max);
}

public int rand_num()
{

  string file_path = System.IO.Path.GetDirectoryName(System.Windows.Forms.Application.ExecutablePath) + @"\Invoices\numbers.txt";
  int randnum = 0;
  string line = "";
  if (File.Exists(file_path)) {
    while(randnum==0)
    {
      randnum = GenerateRandomNo();
      using (System.IO.StreamReader file = new System.IO.StreamReader(file_path)) {
        while ((line = sr.ReadLine()) != null)
        {
           if (line == randnum.ToString()) {
             randnum = 0;
             break;
           }
        }
      }
      file.Close();
    }

    createText = randnum.ToString() + Environment.NewLine;
    File.AppendAllText(file_path, createText);
    file.Close();
    return randnum;
  } else {
    randnum = GenerateRandomNo();
    createText = randnum.ToString() + Environment.NewLine;
    File.WriteAllText(file_path, createText);
    file.Close();
  }
}

PLAESE PAY ATTENTION: Your code base will access multiple time to the text file, this is a solution, but not a best solution. If you will generate all possible random numbers (10000 in you case) the while loop never ends. If you are planning to generate an high number of random numbers this procedure must be changed.

edited Jul 19 '18 at 14:52

OrionMD

389
1
7
13

answered Jul 19 '18 at 03:22

danilonet

1,757
16
33

Don't create new Random object each time you want to sample. – David Heffernan Jul 19 '18 at 03:30
Will your given code work if Textfile is Empty? I mean that when i run this code first time when textfile has no numbers in it. – yaseen enterprises Jul 19 '18 at 03:31
@yaseenenterprises - This is a terrible answer - it won't work as it is. Don't mark it as accepted until you test it. – Enigmativity Jul 19 '18 at 03:32
@yaseenenterprises - It's also a terrible answer in that it's loading the file for ever number that it tests. – Enigmativity Jul 19 '18 at 03:34
YES, but it depends form use cases, if you need to generade a small amount of numbers, if you can't store data in other ways,,, we don't know the behavoiur. I'll try to fix what i have in my hand, i don't want to change the world! – danilonet Jul 19 '18 at 03:36
@yaseenenterprises i fixed the code for new file case, but i repeat consider vaild this answer only if you have to generate a small amount of random numbers in your range – danilonet Jul 19 '18 at 03:51
i want to generate more then 100000 unique numbers. – yaseen enterprises Jul 19 '18 at 03:54
@yaseen you want to generate that many unique values in the range 0 to 9999? – David Heffernan Jul 19 '18 at 05:20

Will this always generate UNIQUE RANDOM number each time?

5 Answers5