How "good" is this method for generating random numbers?

Question

I was searching on google for RNGCryptoServiceProvider with examples on how to limit the range between Max and Min, and still get an even distribution. Before I used modulo operator, but sometimes it I get strange values (above Max)... Anyways this code (credit to unknown) seeds Random with a new seed from RNGCCryptoServiceProvider, everytime the method is called. What do you guys think?

public static int GetRandom(int min, int max)
{
  byte[] b = new byte[sizeof(int)];
  new System.Security.Cryptography.RNGCryptoServiceProvider().GetBytes(b);
  int i = BitConverter.ToInt32(b, 0);
  Random r = new Random(i);
  return r.Next(min, max);
}

It's bad practice to re-seed your rng every time you call it. — recursive, May 17 '13 at 19:34

score 5 · Answer 1 · answered May 17 '13 at 19:37

5

There is no point in using an encryption class random generator to seed a regular random generator. (By the principle of the weakest link...) Just use a single instance of the random generator, and reuse it:

private static Random rnd = new Random();

public static int GetRandom(int min, int max) {
  return rnd.Next(min, max);
}

answered May 17 '13 at 19:37

Guffa

687,336
108
737
1,005

Hm, I was hoping it could improve the Random.Next() as it's quite handy, even though it's week. – Half_Baked May 17 '13 at 19:42
2

The standard `Random` is only weak if you're using it for cryptography or when you need very long periods (e.g. Online Poker games). Otherwise its fine. – Matthew Watson May 17 '13 at 19:45
@MatthewWatson Ok, I'll just stick to it then – Half_Baked May 17 '13 at 19:49

score 3 · Accepted Answer · answered May 17 '13 at 20:03

You'll want to create your RNGCryptoServiceProvider object once and then re-use that object every time you want a new random number. For instance, you can either pass said object into your GetRandom() method or store it in a class-level field.

As far as the RNGCryptoServiceProvider type itself, it generates good numbers on its own, there's no need to create a Random object and pass in a seed. This should give you a very decent distribution:

public static int GetRandom(RNGCryptoServiceProvider rngProvider, int min, int max)
{
    byte[] b = new byte[sizeof(UInt32)];
    rngProvider.GetBytes(b);
    double d = BitConverter.ToUInt32(b, 0) / (double)UInt32.MaxValue;
    return min + (int)((max - min) * d);
}

There is an integer overflow in this implementation if `max - min` is too big to be contained in an int. — mschwaig, Jul 21 '16 at 18:31

score 0 · Answer 3 · answered May 17 '13 at 21:13

It's a better practice to seed your random number generator just one time in your application. I suggest you create a static class for random number generation. The random generation object can yield even distribution just by normal usage. I don't know whats the benefit of seeding the generator with RNGCryptoServiceProvider. I prefer using the time as seeding method as usual. Therefore the following code is my suggestion:

int randomNumber=Rand.get(min,max);

public static class Rand
{
    private static Random rnd;
    static rand()
    {
        rand=new Random(DateTime.Now.Ticks);
    }
    public static int get(int min,int max)
    {
        return rnd.Next(min,max);
    }
}

How "good" is this method for generating random numbers?

3 Answers3

Linked