reiddraper / simple-check

QuickCheck for Clojure
http://reiddraper.github.io/simple-check/
286 stars 18 forks source link

generators/rand-range's range is only half that of JVM integers #37

Closed cemerick closed 10 years ago

cemerick commented 11 years ago

The underlying PRNGs (whether in Java or JavaScript) only provide a ranged generator method between [0,Integer/MAX_VALUE), but the current impl of rand-range can overflow that due to its summing of the bounds' absolute values to determine the upper bound passed to the PRNG:

user=> (gen/sample (gen/choose Integer/MIN_VALUE Integer/MAX_VALUE))
IllegalArgumentException Value out of range for int: 4294967296  clojure.lang.RT.intCast (RT.java:1115)
user=> (gen/sample (gen/choose 0 Integer/MAX_VALUE))
IllegalArgumentException Value out of range for int: 2147483648  clojure.lang.RT.intCast (RT.java:1115)

I think aping data.generators' uniform here would be a good improvement; it floors a random double to scale the diff between the range bounds, and adds that to the lower bound, so it can yield the full range of JVM longs.

Does this sound reasonable/good?

reiddraper commented 11 years ago

Yeah I think that all makes sense. Wanna take a stab at a patch?

cemerick commented 11 years ago

Yeah, I have an impl done. Will extract it from the current project shortly.

cemerick commented 11 years ago

Sorry for the premature closing, accidentally pushed to the wrong master (fixed within 10s).

Turns out data.generators' uniform has the same problem as rand-range, didn't read it closely enough the first time. The associated PR #42 contains an impl that works for / emits the full range of longs.