2

I have a method below for randomly generating alphanumeric combinations. If I call it in a loop it returns the same combination but if I place a break point on GetVoucherNumber and step through it a different number is generated every time. Could you please explain why this happens and how to avoid it?

Code:

static void Main(string[] args)
{
    for (int i = 0; i < 100; i++)
        Console.WriteLine(GetVoucherNumber(6));

    Console.ReadLine();
}

public static string GetVoucherNumber(int length)
{
    var chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
    var random = new Random();
    var result = new string(
        Enumerable.Repeat(chars, length)
                  .Select(s => s[random.Next(s.Length)])
                  .ToArray());

    return result;
}
Denys Wessels
  • 16,829
  • 14
  • 80
  • 120
  • 4
    Could you not just use a GUID in your solution – theedam Apr 11 '14 at 11:52
  • 1
    Please read: http://csharpindepth.com/Articles/Chapter12/Random.aspx – Tim Schmelter Apr 11 '14 at 11:55
  • have you read this http://stackoverflow.com/questions/1654887/random-next-returns-always-the-same-values – Hunyo Silaw Apr 11 '14 at 11:58
  • Just a note, if you can try to avoid similar characters like: O, 0, 1, I, ... it may save typos when users enter the codes... – HonzaCZE Apr 11 '14 at 12:01
  • If you really care about randomness, use a cryptographic random number generator like those deriving from [RandomNumberGenerator](http://msdn.microsoft.com/en-us/library/System.Security.Cryptography.RandomNumberGenerator(v=vs.110).aspx) – Panagiotis Kanavos Apr 11 '14 at 12:04
  • One more idea though, it could happen that some disturbing word will be generated. It would be great to check against some "worldwide dictionary" or to limit the chance of generating the sequence by appearance of max 2 characters in sequence then place numbers and continue, at least like that. Or to moderate the results... – HonzaCZE Apr 11 '14 at 12:11

2 Answers2

14

You're problem is that you are creating only one Random instance. By default, if you do not supply a seed number, the current time will be used as a seed. However, you're code is executing so quickly, that the same time is being used as a seed (because you're code is executing faster than the smallest resolution of the clock), so the number you receive is the same each time.

When you add a breakpoint, you add a delay into your program so that a different time is being used for the seed.

The best fix is to create only a single instance of Random. Each call to random.Next will then generate a different number. For example, you could change your code to:

private static Random random = new Random();

public static string GetVoucherNumber(int length)
{
    var chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
    var result = new string(
        Enumerable.Repeat(chars, length)
                  .Select(s => s[random.Next(s.Length)])
                  .ToArray());

    return result;
}
RB.
  • 36,301
  • 12
  • 91
  • 131
  • Can you please elaborate how creating a single instance of Random helps in this case where different seed is not generated as the code executes faster? – Robert Langdon Apr 11 '14 at 11:58
  • `Random` is not thread safe, you might need a lock or `ThreadLocal`: http://stackoverflow.com/questions/767999/random-number-generator-only-generating-one-random-number/768001#768001 – Tim Schmelter Apr 11 '14 at 11:59
  • @TimSchmelter He does not state he is using threads, although I agree that is a useful thing to note for other people who find this question. – RB. Apr 11 '14 at 12:00
  • 1
    @GrowWithWPF I've tweaked my question slightly to hopefully clarify that - let me know if it's still not clear, and in what way it is not clear and I will update it. – RB. Apr 11 '14 at 12:01
3

Problem:

Random object is created in so successive instance, that it generates same random numbers.

When you do not provide any seed value to random object, current DateTime is used as its seed value. Now when a loop is called, DateTime remains same and hence current DateTime is used to generate random number giving same random numbers.

Solution:

Now Random number instance is created once and its Next is called always, giving actual random numbers.

static void Main(string[] args)
{
    Random random = new Random();
    for (int i = 0; i < 100; i++)
        Console.WriteLine(GetVoucherNumber(6, random));

    Console.ReadLine();
}

public static string GetVoucherNumber(int length, Random random)
{
    var chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
    var result = new string(
        Enumerable.Repeat(chars, length)
                  .Select(s => s[random.Next(s.Length)])
                  .ToArray());

    return result;
}
Nikhil Agrawal
  • 47,018
  • 22
  • 121
  • 208