RNGCryptoServiceProvider with minimum and maximum include

I need a method that returns random numbers from min to max, both numbers inclusive. I found code in the .NET article Matters: Tales from the CryptoRandom by Steven Toub and Sean Farkas where the method would look something like this:

// Note - maxValue is excluded!
public static int GetRandomIntBetween(int minValue, int maxValue)
{
    if (minValue > maxValue) throw new ArgumentOutOfRangeException("minValue");
    if (minValue == maxValue) return minValue;

    var rng = new RNGCryptoServiceProvider();
    var uint32Buffer = new byte[4];
    long diff = maxValue - minValue;

    while (true)
    {
        rng.GetBytes(uint32Buffer);
        uint rand = BitConverter.ToUInt32(uint32Buffer, 0);
        const long max = (1 + (long)int.MaxValue);
        long remainder = max % diff;
        if (rand < max - remainder)
        {
            return (int)(minValue + (rand % diff));
        }
    }
}

      

My attempt at making maxValue inclusive:

public static int GetRandomIntBetween(int minValue, int maxValue)
{
    if (minValue > maxValue) throw new ArgumentOutOfRangeException("minValue");
    if (minValue == maxValue) return minValue;

    // Make maxValue inclusive.
    maxValue++;

    var rng = new RNGCryptoServiceProvider();
    var uint32Buffer = new byte[4];
    long diff = maxValue - minValue;

    while (true)
    {
        rng.GetBytes(uint32Buffer);
        uint rand = BitConverter.ToUInt32(uint32Buffer, 0);
        const long max = (1 + (long)int.MaxValue);
        long remainder = max % diff;
        if (rand < max - remainder)
        {
            return (int)(minValue + (rand % diff));
        }
    }
}

      

It looks strange, but it seems that I can keep the first two checks as they are and still works, although the semantics are slightly different. Also the data results look good. Am I missing something, or is my change normal?

PS - I'm asking this because generating random numbers is obviously quite a delicate task and would like to be sure my approach is correct.

+3


source to share


2 answers


Your changes are correct afaik, the random integer between [a,b]

is the random integer between [a,b+1[

.

As long as maxValue is not int.MaxValue then ++ will overflow, so it would be safer not to change maxValue and move the change to diff computation:



long diff = (long)maxValue - minValue + 1;

      

However, the second check of the original function is obviously wrong if the minValue == maxValue

return minValue is not just a value between minValue and maxValue.

+1


source


See my solution (click there)

You can add an additional class to the class:



public int NextInclusive(int minValue, int maxValue) {
        return Next(minValue, maxValue + 1);
}

      

0


source







All Articles