Skip to content

Commit 498e59b

Browse files
mrbackendmrbackend
committed
fj.test.Rand.reseed() is broken (#237)
1 parent 5c6a4d1 commit 498e59b

File tree

3 files changed

+113
-54
lines changed

3 files changed

+113
-54
lines changed

‎quickcheck/src/main/java/fj/test/Property.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ else if (x.isFalsified()) {
184184
}
185185

186186
/**
187-
* Checks this property using a {@link Rand#Rand(F, F) standard random generator} and the given
187+
* Checks this property using a {@link Rand#standard standard random generator} and the given
188188
* arguments to produce a result.
189189
*
190190
* @param minSuccessful The minimum number of successful tests before a result is reached.
@@ -226,7 +226,7 @@ public CheckResult check(final Rand r, final int minSize, final int maxSize) {
226226
}
227227

228228
/**
229-
* Checks this property using a {@link Rand#Rand(F, F) standard random generator}, 100 minimum
229+
* Checks this property using a {@link Rand#standard standard random generator}, 100 minimum
230230
* successful checks, 500 maximum discarded tests and the given arguments to produce a result.
231231
*
232232
* @param minSize The minimum size to use for checking.
@@ -239,7 +239,7 @@ public CheckResult check(final int minSize,
239239
}
240240

241241
/**
242-
* Checks this property using a {@link Rand#Rand(F, F) standard random generator}, 100 minimum
242+
* Checks this property using a {@link Rand#standard standard random generator}, 100 minimum
243243
* successful checks, 500 maximum discarded tests, minimum size of 0, maximum size of 100.
244244
*
245245
* @return A result after checking this property.
@@ -249,7 +249,7 @@ public CheckResult check() {
249249
}
250250

251251
/**
252-
* Checks this property using a {@link Rand#Rand(F, F) standard random generator}, the given minimum
252+
* Checks this property using a {@link Rand#standard standard random generator}, the given minimum
253253
* successful checks, 500 maximum discarded tests, minimum size of 0, maximum size of 100.
254254
*
255255
* @param minSuccessful The minimum number of successful tests before a result is reached.
@@ -272,7 +272,7 @@ public CheckResult minSuccessful(final Rand r, final int minSuccessful) {
272272
}
273273

274274
/**
275-
* Checks this property using a {@link Rand#Rand(F, F) standard random generator}, 100 minimum
275+
* Checks this property using a {@link Rand#standard standard random generator}, 100 minimum
276276
* successful checks, the given maximum discarded tests, minimum size of 0, maximum size of 100.
277277
*
278278
* @param maxDiscarded The maximum number of tests discarded because they did not satisfy
@@ -297,7 +297,7 @@ public CheckResult maxDiscarded(final Rand r, final int maxDiscarded) {
297297
}
298298

299299
/**
300-
* Checks this property using a {@link Rand#Rand(F, F) standard random generator}, 100 minimum
300+
* Checks this property using a {@link Rand#standard standard random generator}, 100 minimum
301301
* successful checks, 500 maximum discarded tests, the given minimum size, maximum size of 100.
302302
*
303303
* @param minSize The minimum size to use for checking.
@@ -320,7 +320,7 @@ public CheckResult minSize(final Rand r, final int minSize) {
320320
}
321321

322322
/**
323-
* Checks this property using a {@link Rand#Rand(F, F) standard random generator}, 100 minimum
323+
* Checks this property using a {@link Rand#standard standard random generator}, 100 minimum
324324
* successful checks, 500 maximum discarded tests, minimum size of 0, the given maximum size.
325325
*
326326
* @param maxSize The maximum size to use for checking.

‎quickcheck/src/main/java/fj/test/Rand.java‎

Lines changed: 89 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
import fj.F;
44
import fj.data.Option;
5-
import static fj.data.Option.some;
65

6+
import java.util.Random;
7+
8+
import static fj.data.Option.none;
9+
import static fj.data.Option.some;
710
import static java.lang.Math.max;
811
import static java.lang.Math.min;
9-
import java.util.Random;
1012

1113
/**
1214
* A random number generator.
@@ -17,9 +19,17 @@ public final class Rand {
1719
private final F<Option<Long>, F<Integer, F<Integer, Integer>>> f;
1820
private final F<Option<Long>, F<Double, F<Double, Double>>> g;
1921

20-
private Rand(final F<Option<Long>, F<Integer, F<Integer, Integer>>> f, final F<Option<Long>, F<Double, F<Double, Double>>> g) {
22+
// TODO Change to F<Long,Rand> when rand(f,g) is removed
23+
private final Option<F<Long, Rand>> optOnReseed;
24+
25+
private Rand(
26+
F<Option<Long>, F<Integer, F<Integer, Integer>>> f,
27+
F<Option<Long>, F<Double, F<Double, Double>>> g,
28+
Option<F<Long, Rand>> optOnReseed) {
29+
2130
this.f = f;
2231
this.g = g;
32+
this.optOnReseed = optOnReseed;
2333
}
2434

2535
/**
@@ -74,65 +84,97 @@ public double choose(final double from, final double to) {
7484
* @param seed The seed of the new random generator.
7585
* @return A random generator with the given seed.
7686
*/
77-
public Rand reseed(final long seed) {
78-
return new Rand(new F<Option<Long>, F<Integer, F<Integer, Integer>>>() {
79-
public F<Integer, F<Integer, Integer>> f(final Option<Long> old) {
80-
return new F<Integer, F<Integer, Integer>>() {
81-
public F<Integer, Integer> f(final Integer from) {
82-
return new F<Integer, Integer>() {
83-
public Integer f(final Integer to) {
84-
return f.f(some(seed)).f(from).f(to);
85-
}
86-
};
87-
}
88-
};
89-
}
90-
}, new F<Option<Long>, F<Double, F<Double, Double>>>() {
91-
public F<Double, F<Double, Double>> f(final Option<Long> old) {
92-
return new F<Double, F<Double, Double>>() {
93-
public F<Double, Double> f(final Double from) {
94-
return new F<Double, Double>() {
95-
public Double f(final Double to) {
96-
return g.f(some(seed)).f(from).f(to);
97-
}
98-
};
99-
}
100-
};
101-
}
102-
});
87+
public Rand reseed(long seed) {
88+
return optOnReseed.<Rand>option(
89+
() -> {
90+
throw new IllegalStateException("reseed() called on a Rand created with deprecated rand() method");
91+
},
92+
onReseed -> onReseed.f(seed));
10393
}
10494

10595
/**
10696
* Constructs a random generator from the given functions that supply a range to produce a
10797
* result.
98+
* <p>
99+
* Calling {@link #reseed(long)} on an instance returned from this method will
100+
* result in an exception being thrown. Use {@link #rand(F, F, F)} instead.
108101
*
109102
* @param f The integer random generator.
110103
* @param g The floating-point random generator.
111104
* @return A random generator from the given functions that supply a range to produce a result.
112105
*/
113-
public static Rand rand(final F<Option<Long>, F<Integer, F<Integer, Integer>>> f, final F<Option<Long>, F<Double, F<Double, Double>>> g) {
114-
return new Rand(f, g);
106+
// TODO Change Option<F<Long,Rand>> optOnReseed to F<Long,Road> onReseed when removing this method
107+
@Deprecated
108+
public static Rand rand(
109+
F<Option<Long>, F<Integer, F<Integer, Integer>>> f,
110+
F<Option<Long>, F<Double, F<Double, Double>>> g) {
111+
112+
return new Rand(f, g, none());
115113
}
116114

115+
/**
116+
* Constructs a reseedable random generator from the given functions that supply a range to produce a
117+
* result.
118+
*
119+
* @param f The integer random generator.
120+
* @param g The floating-point random generator.
121+
* @param onReseed Function to create a reseeded Rand.
122+
* @return A random generator from the given functions that supply a range to produce a result.
123+
*/
124+
public static Rand rand(
125+
F<Option<Long>, F<Integer, F<Integer, Integer>>> f,
126+
F<Option<Long>, F<Double, F<Double, Double>>> g,
127+
F<Long, Rand> onReseed) {
117128

118-
private static final F<Long, Random> fr = new F<Long, Random>() {
119-
public Random f(final Long x) {
120-
return new Random(x);
121-
}
122-
};
129+
return new Rand(f, g, some(onReseed));
130+
}
123131

124132
/**
125133
* A standard random generator that uses {@link Random}.
126134
*/
127-
public static final Rand standard = new Rand(seed -> from -> to -> {
128-
final int min = min(from, to);
129-
final int max = max(from, to);
130-
final Random random = seed.map(fr).orSome(new Random());
131-
return (int) ((random.nextLong() & Long.MAX_VALUE) % (1L + max - min)) + min;
132-
}, seed -> from -> to -> {
133-
final double min = min(from, to);
134-
final double max = max(from, to);
135-
final Random random = seed.map(fr).orSome(new Random());
136-
return random.nextDouble() * (max - min) + min;
137-
});
135+
public static final Rand standard = createStandard(new Random());
136+
137+
private static Rand createStandard(Random defaultRandom) {
138+
return rand(
139+
optSeed -> from -> to ->
140+
standardChooseInt(optSeed.<Random>option(() -> defaultRandom, Random::new), from, to),
141+
optSeed -> from -> to ->
142+
standardChooseDbl(optSeed.<Random>option(() -> defaultRandom, Random::new), from, to),
143+
newSeed -> createStandard(new Random(newSeed)));
144+
}
145+
146+
/*
147+
* Returns a uniformly distributed value between min(from,to) (inclusive) and max(from,to) (inclusive).
148+
*/
149+
private static int standardChooseInt(Random random, int from, int to) {
150+
int result;
151+
if (from != to) {
152+
int min = min(from, to);
153+
int max = max(from, to);
154+
long range = (1L + max) - min;
155+
long bound = Long.MAX_VALUE - (Long.MAX_VALUE % range);
156+
long r = random.nextLong() & Long.MAX_VALUE;
157+
while (r >= bound) {
158+
// Ensure uniformity
159+
r = random.nextLong() & Long.MAX_VALUE;
160+
}
161+
result = (int) ((r % range) + min);
162+
} else {
163+
result = from;
164+
}
165+
return result;
166+
}
167+
168+
/*
169+
* Returns a uniformly distributed value between min(from,to) (inclusive) and max(from,to) (exclusive)
170+
*
171+
* In theory, this differs from the choose() contract, which specifies a closed interval.
172+
* In practice, the difference shouldn't matter.
173+
*/
174+
private static double standardChooseDbl(Random random, double from, double to) {
175+
double min = min(from, to);
176+
double max = max(from, to);
177+
return ((max - min) * random.nextDouble()) + min;
178+
}
179+
138180
}

‎quickcheck/src/test/java/fj/test/TestRand.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package fj.test;
22

3+
import fj.Equal;
34
import fj.Ord;
5+
import fj.data.List;
46
import fj.data.Stream;
7+
import org.junit.Assert;
58
import org.junit.Test;
69

710
import static org.junit.Assert.assertTrue;
@@ -21,4 +24,18 @@ public void testRandLowHighInclusive() {
2124
assertTrue(s.head() == min && s.last() == max);
2225
}
2326

27+
@Test
28+
public void testReseed() {
29+
Rand rand1 = Rand.standard.reseed(42);
30+
List<Integer> s1 =
31+
List.range(0, 10).map(i -> rand1.choose(Integer.MIN_VALUE, Integer.MAX_VALUE));
32+
33+
Rand rand2 = rand1.reseed(42);
34+
List<Integer> s2 =
35+
List.range(0, 10).map(i -> rand2.choose(Integer.MIN_VALUE, Integer.MAX_VALUE));
36+
37+
assertTrue(s1.zip(s2).forall(p -> p._1().equals(p._2())));
38+
Assert.assertFalse(s1.allEqual(Equal.intEqual));
39+
}
40+
2441
}

0 commit comments

Comments
 (0)