On 15/06/12 06:58, Artur Skawina wrote:
Considering the output of this program:
import std.stdio;
import std.random;
void main() {
foreach (i; 0..20)
writeln(randomSample([0,1,2,4,5,6,7,8,9], 3,
Random(unpredictableSeed)));
}
I'd say the use of std.random should be disallowed on grounds of safety...
Yea, it's for a mix of reasons which Jerro and I prepared fixes for.
In the current RandomSample implementation the first value of the sample is set
in the constructor so that when you call .front() there will be a value waiting.
That means that whatever the subsequent lazy evaluation of the sample, the
first point will _always_ be the same.
You wouldn't expect that to be a problem here because you're generating 20
different samples. But -- nastily -- in the current setup, if you specify a
RNG, its value is set only _after_ the RandomSample object has been constructed,
meaning this RNG does not affect the generation of the first value. Hence the
output you see.
Jerro's fix corrected the copying of RNG. Mine instead delayed the calculation
of the first value so that it was always part of the lazy evaluation. Which is
the better option depends on how you prefer to resolve the inconsistency
identified in this discussion thread, i.e. whether the lazy evaluation should
always come out to the same set of values, or always different ones.
In any case, I think the pull request I have here:
https://github.com/D-Programming-Language/phobos/pull/553
... improves the safety (as well as the speed) of RandomSample, and that pulling
it in would improve the situation without breaking backwards compatibility, but
that still leaves unresolved the inconsistent behaviour identified in this thread.
