Add randomSampleOne #114
Add randomSampleOne #114
Conversation
src/Test/QuickCheck/Gen.purs
Outdated
| generate :: forall a. Gen a -> Effect a | ||
| generate gen = do | ||
| seed <- randomSeed | ||
| pure $ evalGen gen { newSeed: seed, size: 1 } |
There was a problem hiding this comment.
I think 1 is probably too small for the size parameter, as that will probably restrict the values which can come out; we should pick a size large enough to ensure that most/all of the values this generator can produce can be returned here. The existing randomSample uses a size of 10, for instance. I'm not sure whether 10 is a good default here though. Can you give this a try with some common generators and see what you get?
I also think it's not ideal that the name generate doesn't indicate that this function has anything to do with the existing randomSample function, even though they're very similar. I think I'd prefer singleSample or sampleOne or something. Does anyone else have ideas?
There was a problem hiding this comment.
A size of 1 is definitely too small, that'd result in a 1-element array, or one level of structure in a sized recursive generator.
I think 10 seems reasonable, and it won't override a Gen that is resized to more or less than that too. A quick check of some common generators with that size would be good just to double check, but I'd probably have chosen 10 if I were writing it too.
There was a problem hiding this comment.
I also think it's not ideal that the name generate doesn't indicate that this function has anything to do with the existing randomSample function, even though they're very similar. I think I'd prefer singleSample or sampleOne or something. Does anyone else have ideas?
What about randomOne? If one starts typing ran for either one of these functions, it'll show all three of them in the dropdown.
There was a problem hiding this comment.
I don't really like randomOne because I think the "sample" is more important than the "random" in randomSample; what you're doing is sampling from the generator. Perhaps randomSampleOne? That way it is clearly a variant of randomSample, and they'll still all turn up in the completion dropdowns.
src/Test/QuickCheck/Gen.purs
Outdated
| -- | Sample a random generator, using a randomly generated seed | ||
| randomSample' :: forall a. Size -> Gen a -> Effect (Array a) | ||
| randomSample' n g = do | ||
| seed <- randomSeed | ||
| pure $ sample seed n g | ||
|
|
||
| -- | Get a random sample of 10 values | ||
| -- | Get a random sample of 10 values. For a single value, use `randomOne`. |
There was a problem hiding this comment.
This comment needs updating now
Fixes #113