[Mono-dev] Issues with System.Random

[Andreas Nahr]

In Text

>Andreas Nahr wrote:
>> I won't comment on the algorithm itself (keep in mind that the existing
one
>> already was replaced once with a "better" one which failed miserably in
real
>> world apps, so had to be reverted).
>I tested a sequence of 68 million 32-bit values for randomness using the
Diehard test suite. Of course this is only a 
>heuristic indicator that the sequence has good random characteristics, not
a proof. However, the current implementation 
>does not pass (some of) those tests, i.e. there exist cases where it
exhibits bad random characteristics.

As already said I didn't test and can't comment but keep in mind that the
Random class is used (real-world) under VERY specific situations and none of
them are near random. That starts with the initialization to
Environment.Tickcount which IS going to deliver a "relatively" small set of
states in 99% of all cases. On the other hand an initialization with 0 or -1
is probably just about never going to happen (Because it would only happen
for 2 Milliseconds after 50 days of continuous computer uptime).

>> But your patch adds errors for exceptions (which were mostly correct
>> before).
>What errors are you referring to? As far as I can see, all exceptions
mandated by the specification of System.Random are 
>thrown (and tested).
>By the way, I forgot the "Locale.GetText()" for the exception messages.

+				throw new ArgumentNullException ("buffer is
null.");

is clearly an error. First parameter is the argument name, not some
descriptive text.

>> And the unit-tests are no "unit-tests". They don't test the
>> implementation against the specification, they test the implementation
>> against the implementation which is useless.
>
>I disagree. The expected values were not generated by my implementation,
but by this reference implementation of the 
>algorithm (compiled for 32-bit machines):

I can understand your point, but this means that your ARE testing
implementation against implementation. It might be good for testing if you
correctly implemented your algorithm, but it is imho not suited to test if
it is a conforming CLR/.Net implementation (in fact if you would use it on
MS.Net it would fail completely and if you use it on current mono it would
also fail).

>> And moreover you removed ALL
>> Random() constructor tests which most likely are the only of relevance to
>> real-world applications.
>
>Yes, I forgot this one. However, there's not much you can test for (except
that it doesn't throw an exception): the 
>state is private, so it can't be checked directly; the value of
Environment.TickCount() might change between reading it 

Well you can at LEAST test all the provided Next() methods with something
similar than you have written.

Greetings
Andreas