Description
With QCheck2 still not yet released (and still being tested) we have a brief opportunity to discuss the design of its Gen combinators.
Optional parameters
We have had problems with optional parameters in the past (see #75). These may cause problems when they are the only parameters. For QCheck this could usually be solved with type annotations:
utop # QCheck.Gen.small_string;;
- : ?gen:char QCheck.Gen.t -> string QCheck.Gen.t = <fun>
utop # QCheck.Gen.([small_string; string]);;
- : (?gen:char QCheck.Gen.t -> string QCheck.Gen.t) list = [<fun>; <fun>]
utop # (QCheck.Gen.([small_string; string]) : string QCheck.Gen.t list);;
- : string QCheck.Gen.t list = [<fun>; <fun>]
utop # QCheck.make QCheck.Gen.small_string;;
- : string QCheck.arbitrary =
{QCheck.gen = <fun>; print = None; small = None; shrink = None; collect = None;
stats = []}For Check2 however because QCheck2.Gen.t is opaque, even with the fix in PR #161, the type annotation trick doesn't work anymore:
utop # QCheck2.Gen.small_string;;
- : ?gen:char Gen.t -> string Gen.t = <fun>
utop # QCheck2.Gen.([small_string; string]);;
Error: This expression has type string t but an expression was expected of type
?gen:char t -> string t
utop # (QCheck2.Gen.([small_string; string]) : string QCheck2.Gen.t list);;
Error: This expression has type ?gen:char t -> string t
but an expression was expected of type string t
utop # QCheck2.Test.make QCheck2.Gen.small_string;;
Error: This expression has type ?gen:char Gen.t -> string Gen.t
but an expression was expected of type 'a Gen.tEffectively, this means the (labeled) parameter is required, not optional.
As such, we should seriously consider changing QCheck2.Gen.small_string to one of the following:
val small_string : string t (* just use char *)
val small_string : char t -> string t (* make it a regular argument *)
val small_string : gen:char t -> string t (* make it an actual labelled argument *)
...Of the three above, I would prefer one of the first two.
Based on our choice, we should consider whether to update string_size accordingly following the principle of least surprise:
val string_size : ?gen:char t -> int t -> string tLooking at the Gen module's signature I can see a similar problem with Gen.pint : ?origin:int -> int t which we also need to address:
utop # QCheck2.Test.make QCheck2.Gen.pint;;
Error: This expression has type ?origin:int -> int Gen.t
but an expression was expected of type 'a Gen.tHere, we probably should go with a regular or labelled argument. Again, from the principle of least surprise,
we should consider updating the remaining origin-taking combinators to take the same kind of origin argument:
val int_range : ?origin:int -> int -> int -> int t
val float_bound_inclusive : ?origin:float -> float -> float t
val float_bound_exclusive : ?origin:float -> float -> float t
val float_range : ?origin:float -> float -> float -> float t
val char_range : ?origin:char -> char -> char -> char tBesides theGen.generate* bindings, only two bindings now stand a bit out:
val opt : ?ratio:float -> 'a t -> 'a option t
val make_primitive : gen:(Random.State.t -> 'a) -> shrink:('a -> 'a Seq.t) -> 'a tI think for opt the use of an optional parameter is warranted.
For make_primitive we may consider whether labelled arguments are needed.
Generator names:
Naming-wise, we should consider the consistency. Here are the string generators:
val string_size : ?gen:char t -> int t -> string t
val string : string t
val string_of : char t -> string t
val string_readable : string t
val small_string : ?gen:char t -> string tWouldn't string_small be nice and consistent - and easier to find using tab-completion?
I think the same goes for the list generators (small_list -> list_small):
val list : 'a t -> 'a list t
val small_list : 'a t -> 'a list t
val list_size : int t -> 'a t -> 'a list t
val list_repeat : int -> 'a t -> 'a list tand the array generators (small_array -> array_small):
val array : 'a t -> 'a array t
val array_size : int t -> 'a t -> 'a array t
val small_array : 'a t -> 'a array t
val array_repeat : int -> 'a t -> 'a array tFor int*, float, and char generators it is harder...
Use of underscore in name suffixes
I spotted an unfortunate variation in the use of _l, _a or l, a suffixes:
val oneof : 'a t list -> 'a t
val oneofl : 'a list -> 'a t
val oneofa : 'a array -> 'a t
val frequency : (int * 'a t) list -> 'a t
val frequencyl : (int * 'a) list -> 'a t
val frequencya : (int * 'a) array -> 'a t
val shuffle_a : 'a array -> 'a array t
val shuffle_l : 'a list -> 'a list t
val shuffle_w_l : (int * 'a) list -> 'a list t
...
val flatten_l : 'a t list -> 'a list t
val flatten_a : 'a t array -> 'a array t
val flatten_opt : 'a t option -> 'a option t
val flatten_res : ('a t, 'e) result -> ('a, 'e) result tI know a lot of this is legacy, but with a clean QCheck2 now is a good time to consider cleaning these things a bit up.