HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Pseudo-random number generator implementation check

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomnumbergeneratorpseudoimplementationcheck

Problem

I was searching for a pseudo-random number generator for C# and stumbled upon Tales from the CryptoRandom by Stephen Toub and Shawn Farkas, so I tried to implement a variation of their code.

The main difference with their implementation is mine returns inclusive ranges (e.g [0,1] and [min,max] instead of [0,1) and [min,max)),

namespace Albireo.SecureRandom
{
    using System;
    using System.Diagnostics.Contracts;
    using System.Security.Cryptography;

    public static class SecureRandom
    {
        private static readonly RNGCryptoServiceProvider Generator = new RNGCryptoServiceProvider();

        public static byte GetByte()
        {
            var buffer = new byte[1];
            Generator.GetBytes(buffer);
            return buffer[0];
        }

        public static double GetDouble()
        {
            var buffer = new byte[8];
            Generator.GetBytes(buffer);
            return BitConverter.ToDouble(buffer, 0);
        }

        public static short GetInt16()
        {
            var buffer = new byte[2];
            Generator.GetBytes(buffer);
            return BitConverter.ToInt16(buffer, 0);
        }

        public static short GetInt16(short minimum, short maximum)
        {
            Contract.Requires(minimum () >= minimum, "result >= minimum");
            Contract.Ensures(Contract.Result() (minimum () >= minimum, "result >= minimum");
            Contract.Ensures(Contract.Result() (minimum () >= minimum, "result >= minimum");
            Contract.Ensures(Contract.Result() () >= 0, "result >= 0");
            Contract.Ensures(Contract.Result() <= 1, "result <= 1");

            var buffer = new byte[8];
            Generator.GetBytes(buffer);
            return (BitConverter.ToUInt64(buffer, 0) & 0x7FFFFFFFFFFFFFFF) / (decimal) long.MaxValue;
        }
    }
}


I see there's already a similar question, but it lacks the actual implementation.

Since I'm not really skilled in mathematics, the main answers I'm

Solution

Your code is buggy in the range it pulls from the Sample() method. The easiest way to describe this bug is with the code: public static int GetInt32(int minimum, int maximum). For example, if I run:

GetInt32(0, 1)


I would expect 0 half the time, and 1 the other half.

Unfortunately, this is the implementing code:

return (int) (minimum + (Sample() * (maximum - minimum)));


which translates to:

return (int) (0 + (Sample() * (1)));


or, simply:

return (int)(Sample());


Now, sample is a Decimal value from 0.0 to 1.0 inclusive (assuming you got your Sample() method right...)

The cast-to-int will do a decimal truncation on the Sample, and reduce it to an int of 1, if the Sample was 1, and 0 if the sample was anything else...... (somewhere from 0.0 inclusive to 1.0 exclusive).

Which means there is a \$\frac{long.MaxValue - 1}{long.MaxValue}\$ chance the value will be 0, and a \$\frac{1}{long.MaxValue}\$ chance it is 1.

you should run the example code, and see if you ever get the value 1 for: GetInt32(0,1).

I ran your code here on Ideone and it shows poor distribution....

Code Snippets

GetInt32(0, 1)
return (int) (minimum + (Sample() * (maximum - minimum)));
return (int) (0 + (Sample() * (1)));
return (int)(Sample());

Context

StackExchange Code Review Q#55824, answer score: 3

Revisions (0)

No revisions yet.