Rearrange parameters for better chaining.

[ghost]

Fixes #265.

The goal is to allow better data flow through the combinators, like:

Gen.alpha
|> Gen.tryWith (fun exn -> ...)
|> Gen.bind (fun ch -> ...)
|> Gen.map (fun ch -> ...)

[TysonMN]

The functions map2, map3, and map4 in Gen are not necessary. Instead, Gen.apply

fsharp-hedgehog/src/Hedgehog/Gen.fs

Lines 46 to 48 in 7058b4c

	let apply (gf : Gen<'a -> 'b>) (gx : Gen<'a>) : Gen<'b> =
	bind gf <| fun f ->
	bind gx <| (f >> constant)

should have its arguments reversed to

let apply (gx : Gen<'a>) (gf : Gen<'a -> 'b>) : Gen<'b> =

Then given

let flip f b a = f a b
let g = (Gen.constant 1)

we can replace

(fun a b c d e -> a + b + c + d + e)
|> flip Gen.map g
|> Gen.map4 g g g g

with

(fun a b c d -> a + b + c + d)
|> flip Gen.map g
|> Gen.apply g
|> Gen.apply g
|> Gen.apply g

Clearly the advantage of the latter is that it scales to a (curried) function with any number of inputs.

Just as I explained in #265 (comment), the arguments to the function named apply should be flipped compared to the apply operator (which is denoted by <*> in Haskell).

I am not actually requesting that map2, map3, and map4 be removed. I would remove them, but they can stay if others want them.

Instead, I am specifically requesting that the arguments to Gen.apply be swapped.

Also, I find it annoying to call flip before map in both of those cases shown above. In my personal code, I thought of a solution that I like. I created a function called apply1 that is equivalent to flip map. Then the code would look like

(fun a b c d -> a + b + c + d)
|> Gen.apply1 g
|> Gen.apply g
|> Gen.apply g
|> Gen.apply g

Therefore, I also suggest adding this apply1 function.

[ghost]

@TysonMN There are a few ways to skin that cat, you could also write the applicative chain like this:

let g = Gen.constant 1

Gen.success (fun a b c d -> a + b + c + d)
|> Gen.apply g
|> Gen.apply g
|> Gen.apply g
|> Gen.apply g

Or like this:

let (<*>) = Gen.apply
let (<!>) = Gen.map
let g = Gen.constant 1

(fun a b c d -> a + b + c + d) <!> g <*> g <*> g <*> g

Gen.apply would have to be adjusted to make this work, as you've noted. Is there a use case that apply1 would cover that above doesn't?

Also on the topic of applicatives, we should probably add support for the new applicative syntax in F# 5 at some point.

[TysonMN]

Or like this:

let (<*>) = Gen.apply
let (<!>) = Gen.map
let g = Gen.constant 1

(fun a b c d -> a + b + c + d) <!> g <*> g <*> g <*> g

Indeed. In fact, I didn't realize that those operators are already defined.

fsharp-hedgehog/src/Hedgehog/Gen.fs

Lines 507 to 510 in 7058b4c

	[<AutoOpen>]
	module GenOperators =
	let (<!>) = Gen.map
	let (<*>) = Gen.apply

[TysonMN]

Gen.success (fun a b c d -> a + b + c + d)
|> Gen.apply g
|> Gen.apply g
|> Gen.apply g
|> Gen.apply g

I think you mean Gen.constant instead of Gen.success. Yes, that would also work instead of using apply1.

[ghost]

I think you mean Gen.constant instead of Gen.success. Yes, that would also work instead of using apply1.

Ah, yes you're correct. It was literally two lines before 😄

[ghost]

@TysonMN I've flipped the parameters for apply, then I forgot to update the <*> operator so I've fixed that as well and put some regression tests in place.

[TysonMN]

Yes, those changes look good. Thanks @adam-becker! :)

[moodmosaic]

This branch has conflicts that must be resolved, then we can merge it.

[ghost]

@moodmosaic Going to do a quick skim to see if there are other functions that need to be rearranged.

Edit: Ready to merge!

[TysonMN]

Double check if the map operator has its inputs ordered correctly. I think the desired order should allow

Gen.bool <!> not <!> not

while the function form would look like

Gen.bool |> Gen.map not |> Gen.map not

[ghost]

Double check if the map operator has its inputs ordered correctly. I think the desired order should allow

Gen.bool <!> not <!> not

while the function form would look like

Gen.bool |> Gen.map not |> Gen.map not

I've always seen map defined the exact same way as <!>. It allows for applicative programming this way, a la

let y = // Gen<int>
    (fun a b -> a + b) // int -> int -> int
    <!> Gen.constant 1 // Gen<int -> int>
    <*> Gen.constant 2 // Gen<int>

I'm not entirely sold on using the operator versions anyways, and I think they should definitely not be marked with [<AutoOpen>].

I've looked for references, but almost everywhere I looked omitted the <!> operator. Instead I see the ordering you've suggested exposed as |>> (FParsec, FSharpPlus). This would turn your example into:

Gen.bool |>> not |>> not

We can consider adding this operator, but as I said above I think using these operators should be something a user opts in to.

Edit: I think the we only have <!> in F# is because it's illegal syntax to write <$> as in Haskell, where <$> is the functor-enhanced equivalent of $ which is the analog to <| in F#. The meaning of <| is, as we all know, "Apply the expression on the left to the expression on the right", the functor-enhanced version does the same thing, just in functor land. If we look at the definitions, this should clear the air:

( $ ) ::              (a -> b) ->   a ->   b
(<$>) :: Functor f => (a -> b) -> f a -> f b

f  $  x =      f x
f <$> x = fmap f x

val (<| ) : (a -> b) ->     'a  ->     'b
val (<!>) : (a -> b) -> Gen<'a> -> Gen<'b>

let (<| ) f x =         f x
let (<!>) f x = Gen.map f x

[TysonMN]

Ok. You have definitely thought about this more than me. I am fibre with whatever you want. I don't use the operators anyway.

I also think not auto opening the operators is reasonable.

[moodmosaic]

Agreed with @adam-becker. Infix operators in F# must be defined per type (unless you do some funky usage of statically resolved type parameter tricks), so using these operators should be only something a user opts in to.

[ghost]

@moodmosaic I've made the change to opt-in to operators for Gen, I believe this branch is merge-able now.

[moodmosaic]

Looks good to me! Massive piece of work. If @TysonMN also agrees, we can merge 🚀

[TysonMN]

Yep, looks great. Excellent work @adam-becker! :)