Name all arguments and include all type annotations in public API?

[TysonMN]

Do we have to name all arguments and include all type annotations in the public API?

For example, I would prefer to write https://github.com/hedgehogqa/fsharp-hedgehog/blob/569ca6915e77b274ec685ee6b5c339fbfc128bec/src/Hedgehog/Property.fs#L66-L67

as

let map f =
    f |> Outcome.map |> GenTuple.mapSnd |> mapGen

If we decide to change this convention/style, know that we don't have to change the all the code at once. It can happen slowly over time, especially as we are making changes in an area.

[ghost]

The component design guidelines for F# explicitly mention not doing what you're suggesting here. All parameters should be named/typed. If not in the main source file, then in a separate .fsi file once the API stabilizes (which I think we should do as part of a 1.0.0 release).

The reason for this, is that the F# compiler won't generate the same code for point-free style functions. For example, this:

let add x y = x + y

Would compile to a method that looks like:

int add(int x, int y) {
    return x + y;
}

On the other hand, this:

let add x = (+) x

Would compile to a method that looks like:

FSharpFunc<int, int> add(int x) {
    return new FSharpFunc<int, int>(y => x + y);
}

This is not only ugly, it's also very inefficient. Now a simple add function has to allocate an FSharpFunc, and an object for its closure. In the event that the compiler can turn the above into a 2 parameter method, the compiler generated names are utterly useless to consumers of the library (_arg1, _arg2, etc).

I think the code example you provided could just be written simply:

let map f x =
    Outcome.map f x
    |> GenTuple.mapSnd
    |> mapGen

This form is more readable, and also has the virtue of generating better code. My vote is to just write things simply, especially public-facing functions.

[TysonMN]

I think the code example you provided could just be written simply:

let map f x =
    Outcome.map f x
    |> GenTuple.mapSnd
    |> mapGen

This form is more readable, and also has the virtue of generating better code. My vote is to just write things simply, especially public-facing functions.

What you are suggesting doesn't compile.

Here is an alternative suggestion, which uses a backwards pipe.

let map (f : 'a -> 'b) (x : Property<'a>) : Property<'b> =
    f
    |> Outcome.map
    |> GenTuple.mapSnd
    |> mapGen
    <| x

[ghost]

@TysonMN That example was just typed into the comment editor, I think you got the gist of what I was going for. I think we should move type annotations/doc comments to .fsi files. There is a lot of clutter in the source files that could be moved out, and .fsi files are the recommended way of doing this.

[TysonMN]

The component design guidelines for F# explicitly mention not doing what you're suggesting here.

Can you link to that part?

This is not only ugly, it's also very inefficient.

The visual aesthetic of generated code doesn't matter, and I prefer to use a profiler to determine that some code is inefficient (because of such implementation details).

[...] the compiler generated names are utterly useless to consumers of the library ( _arg1 , _arg2 , etc).

Would .fsi files allow us to give better names to these arguments without changing the implementation?

I think you got the gist of what I was going for.

Unfortunately I don't because my code uses a backwards pipe operator, and my impression is that you believe any use of that operator is bad. I still don't know how you would prefer to implement that function in a way that compiles and doesn't use the backwards pipe operator.

[ghost]

The component design guidelines for F# explicitly mention not doing what you're suggesting here.

The last time I read the guidelines, they suggest using .fsi files for public APIs, and those generally prevent the kind of programming you're talking about. Also, I know I'd much rather look at a function like:

let doSomethingInteresting (line: string) =
    line
    |> String.operation
    |> List.concat
    |> List.toSeq

Over

let doSomethingInteresting =
    String.operation
    >> List.concat
    >> List.toSeq

Parameter names, and the occasional type annotation, can really help readability along. And from a user perspective, I'd rather see:

val doSomethingInteresting (line: string) : seq<'a>

Over

val doSomethingInteresting (_arg1: string) : seq<'a>

The visual aesthetic of generated code doesn't matter

Entirely disagree. See above.

and I prefer to use a profiler to determine that some code is inefficient (because of such implementation details).

There are differing levels of inefficiency, at a single function level it's not likely to be perceivable. However, users of this library are liable to have thousands, if not hundreds of thousands of these all chained together and running hundreds of cases per test. Efficiency, even minor improvements, adds up.

Would .fsi files allow us to give better names to these arguments without changing the implementation?

.fsi files just enforce a particular public API, which I think we should use once our API becomes stable (which should be soon). They would prevent "point free" style in most cases at the API level, though I don't have much against it for internal functions.

Unfortunately I don't because my code uses a backwards pipe operator, and my impression is that you believe any use of that operator is bad. I still don't know how you would prefer to implement that function in a way that compiles and doesn't use the backwards pipe operator.

I don't really have an issue with <|, and I use it fairly often in personal projects. Don Syme on the other hand thinks it is one of the worst ideas he's ever had. It was recommended that we not use <| in this codebase, and since we went through the work of removing it, it'd be nice to not add new ones.

[TysonMN]

The component design guidelines for F# explicitly mention not doing what you're suggesting here.

Can you link to that part?

The last time I read the guidelines, they suggest using .fsi files for public APIs, and those generally prevent the kind of programming you're talking about.

Does it still say that? Can you share a link so that we can all get on the same page?

I know I'd much rather look at a function like [...]

Parameter names, and the occasional type annotation, can really help readability along. And from a user perspective, I'd rather see [...]

I agree. I am not suggesting that we eta-reduce as much as possible. I am asking if we should the opposite: should we " Name all arguments and include all type annotations in public API?"

The visual aesthetic of generated code doesn't matter

Entirely disagree. See above.

That is an example of generated documentation, not generated code. Does this observation change your answer?

There are differing levels of inefficiency, at a single function level it's not likely to be perceivable. However, users of this library are liable to have thousands, if not hundreds of thousands of these all chained together and running hundreds of cases per test. Efficiency, even minor improvements, adds up.

It is certainly possible. I am merely suggesting that we optimize code based with guidance from a profiler. This is an industry best practice. I don't think I am saying anything controversial here.

[.fsi files] would prevent "point free" style [...]

Can you share a branch demonstrating this?

Don Syme on the other hand thinks it is one of the worst ideas he's ever had. It was recommended that we not use <| in this codebase, and since we went through the work of removing it, it'd be nice to not add new ones.

Don's opinion notwithstanding, I think a more nuanced relationship with <| is superior.

[ghost]

Does it still say that? Can you share a link so that we can all get on the same page?

It does. Here's the link to the overall guidelines, and here's the portion about .fsi.

I agree. I am not suggesting that we eta-reduce as much as possible. I am asking if we should the opposite: should we " Name all arguments and include all type annotations in public API?"

If that's your suggestion, then 100% yes is the answer. 😅

It is certainly possible. I am merely suggesting that we optimize code based with guidance from a profiler. This is an industry best practice. I don't think I am saying anything controversial here.

Not at all, it's a very reasonable thing. What I was getting at is that some things are obviously slow (such as allocating) and we need to be really careful given the domain we're working in.

Can you share a branch demonstrating this?

Hopefully a small example would suffice. If you need a branch I can certainly get one prepped but it'll have to wait a bit. Consider this .fsi file:

// Example.fsi
module FakeProject.Example

/// Contrived example
val addThenDouble : int -> int -> int

If we try to implement this "point-free":

// Example.fs
module FakeProject.Example

let addThenDouble x =
    (( + ) x) >> (( * ) 2) // spacing around operators because (* and *) are comment tokens.

We'll get an error about not conforming to the public API, which expects addThenDouble to be a function int -> int -> int but is instead a function int -> (int -> int). Even though they are semantically equivalent, they aren't equivalent in practice, thus the error.

error FS0034: Module 'FakeProject.Example' contains
    val addThenDouble : x:int -> (int -> int)    
but its signature specifies
    val addThenDouble : int -> int -> int    
The arities in the signature and implementation differ. The signature specifies that 'addThenDouble' is function definition or lambda expression accepting at least 2 argument(s), but the implementation is a computed function value. To declare that a computed function value is a permitted implementation simply parenthesize its type in the signature, e.g.
    val addThenDouble: int -> (int -> int)
instead of
    val addThenDouble: int -> int -> int.

Notice that in the example .fsi file we can put doc comments, type annotations, etc and not clutter the implementation with them.

That is an example of generated documentation, not generated code. Does this observation change your answer?

Erm, the example I provided was for generated code. However, I think we're arguing in favor of the same things so this might be a moot point.

Don's opinion notwithstanding, I think a more nuanced relationship with <| is superior.

This might need more discussion, and is separate from the issue at hand. My feeling is that <| has its uses but |> should be preferred. I've used <| to separate long arguments to functions, a la:

someFunction
    <| arg1 // These aren't long, but you get the idea
    <| arg2
    // ...

But a strong argument can be made that these arguments should just be bound to a name instead :)

Maybe a good starting point would be to define where <| is acceptable to us? Start a discussion if that's something you think could be productive.