Guid.NewGuid() VS a random string generator from Random.Next()

Question

My colleague and I are debating which of these methods to use for auto generating user ID's and post ID's for identification in the database:

One option uses a single instance of Random, and takes some useful parameters so it can be reused for all sorts of string-gen cases (i.e. from 4 digit numeric pins to 20 digit alphanumeric ids). Here's the code:

// This is created once for the lifetime of the server instance class RandomStringGenerator {     public const string ALPHANUMERIC_CAPS = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890";     public const string ALPHA_CAPS = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";     public const string NUMERIC = "1234567890";      Random rand = new Random();     public string GetRandomString(int length, params char[] chars)     {         string s = "";         for (int i = 0; i < length; i++)             s += chars[rand.Next() % chars.Length];          return s;     } } 

and the other option is simply to use:

Guid.NewGuid(); 

see Guid.NewGuid on MSDN

We're both aware that Guid.NewGuid() would work for our needs, but I would rather use the custom method. It does the same thing but with more control.

My colleague thinks that because the custom method has been cooked up ourselves, it's more likely to generate collisions. I'll admit I'm not fully aware of the implementation of Random, but I presume it is just as random as Guid.NewGuid(). A typical usage of the custom method might be:

RandomStringGenerator stringGen = new RandomStringGenerator(); string id = stringGen.GetRandomString(20, RandomStringGenerator.ALPHANUMERIC_CAPS.ToCharArray()); 

Edit 1:

• We are using Azure Tables which doesn't have an auto increment (or similar) feature for generating keys.
• Some answers here just tell me to use NewGuid() "because that's what it's made for". I'm looking for a more in depth reason as to why the cooked up method may be more likely to generate collisions given the same degrees of freedom as a Guid.

Edit 2:

We were also using the cooked up method to generate post ID's which, unlike session tokens, need to look pretty for display in the url of our website (like http://mywebsite.com/14983336), so guids are not an option here, however collisions are still to be avoided.

Answer 1

I am looking for a more in depth reason as to why the cooked up method may be more likely to generate collisions given the same degrees of freedom as a Guid.

First, as others have noted, Random is not thread-safe; using it from multiple threads can cause it to corrupt its internal data structures so that it always produces the same sequence.

Second, Random is seeded based on the current time. Two instances of Random created within the same millisecond (recall that a millisecond is several million processor cycles on modern hardware) will have the same seed, and therefore will produce the same sequence.

Third, I lied. Random is not seeded based on the current time; it is seeded based on the amount of time the machine has been active. The seed is a 32 bit number, and since the granularity is in milliseconds, that's only a few weeks until it wraps around. But that's not the problem; the problem is: the time period in which you create that instance of Random is highly likely to be within a few minutes of the machine booting up. Every time you power-cycle a machine, or bring a new machine online in a cluster, there is a small window in which instances of Random are created, and the more that happens, the greater the odds are that you'll get a seed that you had before.

(UPDATE: Newer versions of the .NET framework have mitigated some of these problems; in those versions you no longer have every Random created within the same millisecond have the same seed. However there are still many problems with Random; always remember that it is only pseudo-random, not crypto-strength random. Random is actually very predictable, so if you are relying on unpredictability, it is not suitable.)

As other have said: if you want a primary key for your database then have the database generate you a primary key; let the database do its job. If you want a globally unique identifier then use a guid; that's what they're for.

And finally, if you are interested in learning more about the uses and abuses of guids then you might want to read my "guid guide" series; part one is here:

http://blogs.msdn.com/b/ericlippert/archive/2012/04/24/guid-guide-part-one.aspx

Answer 2

As written in other answers, my implementation had a few severe problems:

• Thread safety: Random is not thread safe.
• Predictability: the method couldn't be used for security critical identifiers like session tokens due to the nature of the Random class.
• Collisions: Even though the method created 20 'random' numbers, the probability of a collision is not (number of possible chars)^20 due to the seed value only being 31 bits, and coming from a bad source. Given the same seed, any length of sequence will be the same.

Guid.NewGuid() would be fine, except we don't want to use ugly GUIDs in urls and .NETs NewGuid() algorithm is not known to be cryptographically secure for use in session tokens - it might give predictable results if a little information is known.

Here is the code we're using now, it is secure, flexible and as far as I know it's very unlikely to create collisions if given enough length and character choice:

class RandomStringGenerator {     RNGCryptoServiceProvider rand = new RNGCryptoServiceProvider();     public string GetRandomString(int length, params char[] chars)     {         string s = "";         for (int i = 0; i < length; i++)         {             byte[] intBytes = new byte[4];             rand.GetBytes(intBytes);             uint randomInt = BitConverter.ToUInt32(intBytes, 0);             s += chars[randomInt % chars.Length];         }         return s;     } }