Skip to content

Conversation

@cartermp
Copy link
Contributor

Addresses #2049

cc @dsyme and @eiriktsarpalis

Eirik, I haven't incorporated the stuff we worked on yet. Thus far, the work has just been in reigning in various bits of information and trying to make it all sound good.

@mairaw mairaw added the WIP label Apr 10, 2018
@dsyme
Copy link
Contributor

dsyme commented Apr 12, 2018

@cartermp Why not make the updates on fsharp.org, and if necessary mirror here?

My gut feeling is that the component design guidelines should be maintained by a community, neutral organisation on a non-Microsoft site. It feels important for the community to be owning this, and being seen to own this.

@cartermp
Copy link
Contributor Author

cartermp commented Apr 12, 2018

@dsyme Reasons are 5-fold, in order of importance:

  1. Lots of feedback from beginners that we need a style/formatting and "how do I do stuff correctly" guide. This is also echoed by feedback from @eiriktsarpalis and others in larger organizations. Such a guide does not exist on fsharp.org. Component Design Guidelines are a part of that, but they are scoped towards a specific thing.

  2. Our site holds more weight and the content is more discoverable. For newcomers especially, having something up to date from a Microsoft domain will be important. Discoverability might be solved by the eventual fsharp.org redesign, but I don't want to be sitting on content that I can distribute more effectively in the interim.

  3. The Component Design Guidelines aren't really being maintained. Last commit is in 2016 with history indicating that the major work came from a Microsoft employee and not a community effort.

  4. I'm not convinced that the visibility of this - guidelines on how to write F# code - is inherently a community thing nor something that the community particularly cares to own. I do legitimately think that the optics of Microsoft also having guidelines for how to write F# code effectively far outweighs any possible negatives (which are likely to be associated with an older Microsoft from a different time).

  5. Since our documentation is open source, I think it's perfectly fine to see this as a collaboration between Microsoft and the community. After all, it is how this effort started.

That said, if there is a plan to keep the CDG on fsharp.org after the redesign and cleanup of the site, I see no reason why this couldn't also be mirrored there.

| UndisclosedFailure -> printfn "Failed: unknown - retry perhaps?"
```

In general, if you can model the different ways that something can **fail** in your domain, then error handling code is no longer treated as something you must deal with in addition to regular program flow. It is simply a part of normal program flow. This also makes it easier to maintain as your domain changes over time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be worth mentioning that explicit error cases are more amenable to unit testing.


## Do not use partial application and currying in public APIs

With little exception, the use of partial application in public APIs can be confusing for consumers. Generally speaking, `let`-bound values in F# code are **values**, not **function values**. Mixing together values and function values can result in saving a very small number of lines of code in exchange for quite a bit of cognitive overhead, especially if combined with operators such as `>>` to compose functions together.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A point to drive through here is that curried functions do not label their arguments, so instead of

val foo : name:string -> age:int -> unit

you get

val foo : (string -> int -> unit)

which can be confusing from a documentation perspective.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommended workaround would be to do full-blown eta-expansion, even if it does feel redudant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Covered.

...
```

### Only use `[<AutoOpen>]` for nested private modules
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an exception to that rule, namely for exposing things like expression builders and extension methods at the namespace level.


First, it makes the API itself reliant on global state. Secondly, it pushes application configuration into the codebase itself. This is very difficult to maintain for larger codebases.

Finally, and this is perhaps the most insidious, is that module initialization compiles into a static constructor for the entire compilation unit. If any error occurs in let-bound value initialization in that module, it manifests as a `TypeInitializationException` which is then cached for the entire lifetime of the application. There is no way to tell what actually went wrong when this happens.
Copy link
Member

@eiriktsarpalis eiriktsarpalis Apr 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth noting that compilation-unit spanning cctors can have interesting performance implications at startup time, as illustrated here: dotnet/fsharp#4465

This enables the following:

1. Pushing any dependent state outside of the API itself.
2. Configuration can now be done outsie of the API.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outside


### Do not use monadic error handling to replace exceptions

It is seen as somewhat taboo in functional programming to use exceptions. Indeed, exceptions violate purity and totality, so it's safe to consider them not-quite functional. However, this ignores the reality of where code must run. Even in purely functional languages like Haskell, programs such as ```2 `div` 0``` produce a runtime exception.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this delves into philosophical discussion, and might not be appropriate for an F# practices guide.

// This is bad!
let tryReadAllText (path : string) =
try System.IO.File.ReadAllText path |> Some
with _ -> None
Copy link
Member

@eiriktsarpalis eiriktsarpalis Apr 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth contrasting with the following:

let tryReadAllText (path : string) =
    try System.IO.File.ReadAllText path |> Some
    with :? FileNotFoundException -> None

Because the above catches a very particular error class, the meaning of None is less ambiguous (provided it has been sufficiently documented in XML docs) as opposed to functioning as a catch-all. Other examples include doing GET requests against restful APIs and catching on http exceptions with 404 status codes (which also illustrates the applicability of when guards in F# exception handling clauses)

```fsharp
val foo : name:string -> age:int -> unit
val foo' : (string -> int -> unit)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to the context of this section, but I would generally advise against the use of ticked variable names because they are generally not recognized by the debugger (admittedly this is working around tooling defects, which could be fixed in future releases)


Type inference can save you from typing a lot of boilerplate. And automatic generalization in the F# compiler can help you write more generic code with almost no extra effort on your part. However, these features are not universally good.

Consider labeling argument names with explicit types in public APIs and do not rely on type inference for this. The reason for this is that **you** should be in control of the shape of your API, not the compiler. Although the compiler can do a fine job at infering types for you, it is possible to have the shape of your API change if the internals it relies on have changed types. This may be what you want, but it will almost certainly result in a breaking API change that downstream consumers will then have to deal with. Instead, if you explicitly control the shape of your public API, then you can control these breaking changes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few other points around generics:

  • Explicitly specifying type arguments in angle brackets can be helpful for a variety of reasons: aids type inference when calling the function, forces naming of the type arguments, prevents runaway generic arguments. Probably avoid in "functional" apis like the List module.
  • More than 2-3 type arguments in a function is code smell.
  • Name your generic arguments. Nothing signals bad API design worse than a generic function exposing an argument assigned the type parameter 'f by the compiler. Sometimes it might make sense to assign a name to a type parameter, like 'Document in the context of a CosmosDB client.
  • Do not use lowercase or camel cased parameter names in generic methods or types.

let dep3 = Random().Next()
let function1 arg = doStuffWith dep1 dep2 dep3 arg
let function2 arg = doSutffWith dep1 dep2 dep3 arg
Copy link
Member

@eiriktsarpalis eiriktsarpalis Apr 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few other notes around module design:

  • Public module functions should rarely accept more than 3-4 arguments.
  • Public module functions should not accept too many function arguments. If they do expose function arguments, their intent should be evident simply by examining the function signature (think hoogle). I cannot understand what this function does : val myCoolFunc : (int -> bool -> string -> int) -> (int -> 'T -> string) -> string * 'T list.
  • Mostly avoid exposing third-order functions or higher, e.g. val peirce : ((string -> unit) -> string) -> string.
  • If your function still requires too many function arguments, consider refactoring so that it accepts an interface. This usually records intention better.
  • If your functions take more than 4-5 arguments, consider converting your module into a static facade, exposing methods that name their individual parameters. Do make use of optional parameters when appropriate.
  • Avoid mixing tupled arguments with curried style, e.g. val foo : (int * string) -> int -> (bool * bool * bool) -> int.
  • Avoid using tuples as return types. Pairs and triples are occasionally acceptable, when the API clearly communicates the purpose of each element type. In most cases however, it might make sense to write a custom record type (or use anonymous records in future F# releases) that clearly labels each of the values.

Copy link
Contributor

@bartelink bartelink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of style/clarity suggestions; I've erred on the side of stating every thought the text provokes - please dont feel the need to ack or apply any of my comments


This document looks at some of the issues related to F# component design and coding. A component can mean any of the following:

* A layer in your an F# project which has external consumers within that project
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your an; which-> that

* A layer in your an F# project which has external consumers within that project
* A library intended for consumption by F# code across assembly boundaries
* A library intended for consumption by any .NET language across assembly boundaries
* A library intended for distribution via a package manager, such as [NuGet](https://nuget.org)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps manager -> repository?


### Add XML documentation comments to your code

XML documents on public APIs ensure that users can get great Intellisense and Quickinfo when using these types and members, and enable building documentation files for the library. See the [XML Documentation](../language-reference/xml-documentation.md) about various xml tags that can be used for additional markup within xmldoc comments.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documents-> documentation


#### Use .NET naming and capitalization conventions

The following table follows .NET naming and caputalization conventions. There are small additions to also include F# constructs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

capitalization

| Construct | Case | Part | Examples | Notes |
|-----------|------|------|----------|-------|
| Concrete types | PascalCase | Noun/ adjective | List, Double, Complex | Concrete types are structs, classes, enumerations, delegates, records, and unions. Though type names are traditionally lowercase in OCaml, F# has adopted the .NET naming scheme for types.
| DLLs | PascalCase | | Fabrikom.Core.dll | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fabrikam

Instead of F# lists:

```fsharp
member this.PrintNames(names : list<string>) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use string list to emphasize ?

member this.TupledArguments(str, num) = String.replicate num str
```

Tip: If you’re designing libraries for use from any .NET language, then there’s no substitute for actually doing some experimental C# and Visual Basic programming to ensure that your libraries look good from these languages. You can also use tools such as .NET Reflector and the Visual Studio Object Browser to ensure that libraries and their documentation appear as expected to developers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look good -> feel right/work well?


* Functions and lists become references to corresponding types in the F# library.

The full rules for how F# types, modules, and members are represented in the .NET Common Intermediary Language are explained in the F# language reference on the F# website.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intermediate

The code below shows how to adjust this code to take these things into account.

```fsharp
namespace ExpertFSharp.Types
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refrain from egregious product placement please ;)


* We adjusted several names: `Point1`, `n`, `l`, and `f` became `RadialPoint`, `count`, `factor`, and `transform`, respectively.

* We used a return type of `seq<RadialPoint>` instead of `RadialPoint list` by changing a list construction using `[ ... ]` to a sequence construction using `seq { ... }`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seq -> IEnumerable ?

@cartermp
Copy link
Contributor Author

cartermp commented Apr 14, 2018

Thanks for the review so far, @eiriktsarpalis and @bartelink - I applied changes based on your feedback. Eirik, I've tracked some of your additional suggestions in the tracking gist (I know, super high quality tool).

It is seen as somewhat taboo in functional programming to use exceptions. Indeed, exceptions violate purity and totality, so it's safe to consider them not-quite functional. However, this ignores the reality of where code must run. Even in purely functional languages like Haskell, programs such as ```2 `div` 0``` produce a runtime exception.

F# runs on .NET, and exceptions are a key piece of .NET. This is not something to ignore or attempt to work around.
It is seen as somewhat taboo in functional programming to use exceptions. Indeed, exceptions violate purity and totality, so it's safe to consider them not-quite functional. However, this ignores the reality of where code must run. F# runs on .NET, and exceptions are a key piece of .NET. This is not something to ignore or attempt to work around.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would remove reference to total functions. Even on a theoretical level, most interesting lambda calculi admit divergence, e.g. let rec foo () = foo ()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on who reads this, the tone of the paragraph might be interpreted as "writing pure and total function is an admirable goal, however the .NET mothership is here to crush your dreams". Would rephrase into something along the lines of "write code on the assumption that most things are neither pure or total, to minimize unpleasant surprises"


## Organizing code

F# features two primarily ways to organize code: modules and namespaces. They are very similar, but do have the following differences:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

primarily-> primary; They are very similar -> These are similar


F# features two primarily ways to organize code: modules and namespaces. They are very similar, but do have the following differences:

* Namespaces are compiled as .NET namespaces. Modules are compared as static classes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compared -> compiled. I would say compiled to, but compiled as, or represented as probably makes sense in some naming scheme

F# features two primarily ways to organize code: modules and namespaces. They are very similar, but do have the following differences:

* Namespaces are compiled as .NET namespaces. Modules are compared as static classes.
* Namespaces are always top-level. Modules can be top-level and nested within other modules.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

top-level is not a meaningful term; I know what this means but doubt it will mean the same thing to every reader. I'm not sure whether you're referring to how the module and namespace statements operate at code level, relate to each other or the net effect in the generated code. "Modules can have child modules 'nested' within them. Namespaces do not nest in this way - compilation artifacts are arbitrarily 'tagged' with a name that happens to bear commonality which is represented as a hierarchy by tooling."


* Namespaces are compiled as .NET namespaces. Modules are compared as static classes.
* Namespaces are always top-level. Modules can be top-level and nested within other modules.
* Namespaces can span mutiple files. Modules cannot.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mutiple


### Carefully apply `[<AutoOpen>]`

The `[<AutoOpen>]` attribute can pollute callers and the answer to where something comes from is "magic". This is generally not a good thing. An exception to this rule is the F# Core Library itself (though this fact is also a bit controversial).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

autoopen attribute -> AutoOpen construct; can pollute callers -> can lead to the user's scope becoming 'polluted' ?

member __.Contains(a, b) = t.ContainsKey(a) && t.[a].Contains(b)
```

Because this class has no members which can change, and its binding is immutable, it is also effectively immutable. Additionally, it safely encapsulates the underlying mutation-based data structure. Classes are a powerful way to encapsulate data and routines which are mutation-based without exposing the details to callers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which -> that x2


### Prefer `let mutable` to reference cells

Reference cells can be a powerful mechanism for pointer arithemetic, but are generally not ideal for other performance-critical code. Consider the following function:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arithemetic. Also I have no idea why they can be "can be a powerful mechanism for pointer arithemetic" ?

let kernels =
let acc = ref Set.empty
ProcessWorkList startKernels (fun kernel ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should ProcessWorkList be camel cased ? (not sure what's being conveyed)

let mutable acc = Set.empty
ProcessWorkList startKernels (fun kernel ->
if not (acc.Contains(kernel) then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing right paren nd/or remove the one after Contains

acc |> Seq.toList
```

Aside from the single point of mutation in the middle of the lambda expression, all other code which works with `acc` no differently than if it were a normal `let`-bound immutable value. This will make it easier to change over time. No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which -> that

all other code which works with acc no differently than if it were a normal

all other code that touches acc can do so in a manner that is no different to the usage of a normal


This article offers guidelines for how to format your code so that your F# code is:

* Generally viewed as easier to read
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally more legible?

This article offers guidelines for how to format your code so that your F# code is:

* Generally viewed as easier to read
* Adhering to formatting tools in Visual Studio and other editors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is in accordance with conventions applied by ... ?


## General rules for indentation

F# uses significant whitespace by default. The following guidelines will help you reign in formatting challenges for this.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will help you reign in formatting challenges for this.

reign -> rein

But really I dont get the point of the sentence from this. Perhaps:

are intended to provide guidance as to how to juggle some challenges this can impose

```fsharp
// OK
type Volume =
| Liter of float
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Litre ;P

// OK
type Volume =
| Liter of float
| USPint of float
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be replaced with a different term - while USPint is FDG compliant it looks really ugly. FluidOunce ?

...
```

In large assemblies and implementation files, PascalCase is sometimes used to indicate major internal routines.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a need to promulgate this as the value of such a scheme is minimal, versus the risk of people using this to further arbitrary inconsistency. There are many better ways to achieve similar demarcation. Next we'll be advocating #regions !

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deaded it


1. **Good F# code is succinct and expressive**

F# has many features which allow you to express actions in fewer lines of code and re-use generic functionality. The F# core library also contains many useful types and functions for working with common collections of data. As a general rule, if you can express a solution to a problem in less lines of code, other developers (or your future self) will be appreciative. It is also highly recommended that you use a library such as FSharp.Core, the [vast .NET libraries](https://docs.microsoft.com/dotnet/api/) which F# runs on, or a third-party package on [NuGet](https://www.nuget.org/) when you need to do a nontrivial task.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which -> that


3. **Good F# code makes use of object programming, not object orientation**

F# has full support for programming with objects in .NET, including [classes](../language-reference/classes.md), [interfaces](../language-reference/interfaces.md), [access modifiers](../language-reference/access-control.md), [abstract classes](../language-reference/abstract-classes.md), and so on. For more complicated functional code, such as functions which must be context-aware, objects can easily encapsulate contextual information in ways that functions cannot. Features such as [optional parameters](../language-reference/members/methods.md#optional-arguments) and [overloading](../language-reference/members/methods.md#overloaded-methods) also aid consuption of this functionality for callers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which -> that
consuption


5. **Good F# code is toolable**

Tools are invaluable for working in large codebases, and there are ways to write F# code such that it can be used more effectively with tools. One example is making sure you don't overdo it with a point-free style of programming, so that intermmediate values can be inspected with a debugger. Another example is using [XML documentation comments](../language-reference/xml-documentation.md) describing constructs such that tooltips in editors can display those comments at the call site. Always think about how your code will be read, navigated, debugged, and manipulated by other programmers with their tools.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intermmediate -> intermediate


The [F# Formatting Guidelines](formatting.md) provide guidance on how to format code so that it is easy to read.

The [F# coding conventions](conventions.md) provide guidance for F# programming idioms which will help the long-term maintenance of larger F# codebases.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which -> that

Instead of functioning as a catch-all, this function will now properly handle the case when a file was not found and assign that meaning to a return value which can map to that error case, while not discarding any contextual information or forcing callers to deal with a case that may not be relevant at that point in the code.
Instead of functioning as a catch-all, this function will now properly handle the case when a file was not found and assign that meaning to a return . This return value can map to that error case, while not discarding any contextual information or forcing callers to deal with a case that may not be relevant at that point in the code.

Types such as `Result<'Success, 'Error>` are appropriate for basic operations where they aren't nested, and F# optional types are perfect for representing when something could either reutrn *something* or *nothing*. They are not a replacement for exceptions, though, and should not be used in an attempt to replace exceptions. Rather, they should be applied judiciously to address specific aspects of exception and error management policy in targeted ways.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reutrn -> return


In addition to the previous table, be aware of the following:

#### Avoid abbreviaitons
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo


### Sort `open` statements topologically

In F#, the order of declarations matters, including with `open` statements. This is unlike C#, where `using` statements can be sorted arbitrarily, where the effect is the same regardless of ordering.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with -> that of

This is unlike C#, where using statements can be sorted arbitrarily, where the effect is the same regardless of ordering.

This is in direct contrast to C#, where the effect of using and using static statements is designed to be independent of the ordering of the statements in the file.

(the existing grammar is broken; I'm not sure it needs to be reworded to this degree)


Note that a line break separates topological layers, with each layer being sorted alphanumerically afterwards. This cleanly organizes code without accidentally shadowing values.

## Use classes to contain values which have side effects
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that

| Function | Syntax | Purpose |
|----------|--------|---------|
| `invalidArg` | `invalidArg "argumentName" "message"` | Raises a `System.ArgumentException` with a specified argument name and message. |
| `nullArg` | `nullArg "argumentName"` | Raises a `System.ArgumentNullException` with the specified argument name. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this up one - ANE is preferred to AE if it pertains

| `invalidArg` | `invalidArg "argumentName" "message"` | Raises a `System.ArgumentException` with a specified argument name and message. |
| `nullArg` | `nullArg "argumentName"` | Raises a `System.ArgumentNullException` with the specified argument name. |
| `invalidOp` | `invalidOp "message"` | Raises a `System.InvalidOperationException` with the specified message. |
|`raise`| `raise ExceptionType("message")` | General purpose mechanism for throwing exceptions. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raise ExceptionType("message")

actually requires parentheses I think? suggest: raise (MyException("Message"))

@cartermp cartermp removed the WIP label Apr 23, 2018
cartermp and others added 6 commits April 24, 2018 09:11
When you have static members, sometimes they mimic module functions. Having capitalization based on implementation details makes for naming inconsistencies when consuming API's in f#.
Also clarified the role of CompiledName attribute. You can see an examples of the attribute usage in https://github.com/fsharp/fsharp/blob/master/src/fsharp/FSharp.Core/option.fs#L52
Adding a section on static members
member v.Y = y
[<CompiledName("Create")>]
static member create x y = Vector (x, y)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a member, the recommendation is always to use a PascalCase name. We would not recommend people write code like this.


#### Use method overloading for member functions, if doing so provides a simpler API

Method overloading is a powerful tool for simplifying an API that may need to do multiple related things.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In F#, it is more common to overload on number of arguments rather than types of arguments. We should probably mention this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this after the code sample, which does overload on number of arguments so as to cement this as the way to do things (generally)

@@ -0,0 +1,873 @@
# F# component design guidelines

This document is a set of component design guidelines for F# programming, based on [the F# Component Design Guidelines, v14, Microsoft Research, and [another version](http://fsharp.org/specs/component-design-guidelines/) originally curated and maintained by the F# Software Foundation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something's wrong here: square brackets are not balanced.

|-----------|------|------|----------|-------|
| Concrete types | PascalCase | Noun/ adjective | List, Double, Complex | Concrete types are structs, classes, enumerations, delegates, records, and unions. Though type names are traditionally lowercase in OCaml, F# has adopted the .NET naming scheme for types.
| DLLs | PascalCase | | Fabrikam.Core.dll | |
| Union tags | PascalCase | Noun | Some, Add, Success | Do not use a prefix in public APIs. Optionally use a prefix when internal, such as `type Teams = TAlpha | TBeta | TDelta.` |
Copy link
Contributor

@ForNeVeR ForNeVeR Apr 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, GitHub doesn't render `type Teams = TAlpha | TBeta | TDelta.` | fragment properly (it got confused about pipes inside of a code fragment inside of a table cell). Will docs.microsoft.com render it?

@cartermp
Copy link
Contributor Author

cartermp commented May 1, 2018

@mairaw and @BillWagner would love to get a review to merge soon (ideally before build)

@BillWagner
Copy link
Member

I will review tonight.

Copy link
Contributor

@mairaw mairaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cartermp. Only reviewed 1st topic so far. Looks good but left some comments to improve it.

@@ -0,0 +1,875 @@
# F# component design guidelines
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new topics are missing the metadata section.

@@ -0,0 +1,875 @@
# F# component design guidelines

This document is a set of component design guidelines for F# programming, based on the F# Component Design Guidelines, v14, Microsoft Research, and [another version](http://fsharp.org/specs/component-design-guidelines/) originally curated and maintained by the F# Software Foundation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use https link?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it renders a bad site, not sure if HTTPS is turned on - @ReedCopsey?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cartermp https should now work, and render correctly. Github pages finally supports it, so we were in the process of switching, and had a resource bug. It's now fixed.


Regardless of the methodology, the component and library designer faces a number of practical and prosaic issues when trying to craft an API that is most easily usable by developers. Conscientious application of the [.NET Library Design Guidelines](../../standard/design-guidelines/index.md) will steer you towards creating a consistent set of APIs that are pleasant to consume.

## General Guidelines
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: sentence case for headings


## Guidelines for F#-Facing Libraries

In this section, we present recommendations for developing public F#-facing libraries; that is, libraries exposing public APIs that are intended to be consumed by F# developers. There are a variety of library-design recommendations applicable specifically to F#. In the absence of specific recommendations below, the .NET Library Design Guidelines are the fallback guidance.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this section, we present recommendations -> This section presents recommendations

remove below or replace with something else like in this article

| let values (external) | camelCase or PascalCase | Noun/verb | List.map, Dates.Today | let-bound values are often public when following traditional functional design patterns. However, generally use PascalCase when the identifier can be used from other .NET languages. |
| Property | PascalCase | Noun/ adjective | IsEndOfFile, BackColor | Boolean properties generally use Is and Can and should be affirmative, as in IsEndOfFile, not IsNotEndOfFile.

In addition to the previous table, be aware of the following:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

following recommendations?


#### Use the TryGetValue pattern instead of returning F# option values (`Option<T>`), and prefer method overloading to taking F# option values as arguments

Common patterns of use for the F# option type in APIs are better implemented in vanilla .NET APIs using standard .NET design techniques. Instead of returning an F# option value, consider using the bool return type plus an out parameter as in the TryGetValue pattern. And instead of taking F# option values as parameters, consider using method overloading or optional arguments.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inline code out so it doesn't get translated?


Likewise, a statistics library might include a module with functions `erf` and `erfc`, where this module is designed to be explicitly or automatically opened.

#### Consider using `[<RequireQualifiedAccess>]` and carefully apply `[<AutoOpen>]` attributes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


This document looks at some of the issues related to F# component design and coding. A component can mean any of the following:

* A layer in your F# project that has external consumers within that project
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add punctuation to each list item

member Radius : double
```

Let’s take a look at how this F# type will appear to a programmer using another .NET language. For example, the approximate C# “signature” is as follows:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will appear -> appears

}
```

The fixes we have made to prepare this type for use as part of a vanilla .NET library are as follows:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove we from here and from the sentences that follow

| (false, _) -> false
```

`Closure1Table` encapsulates the underlying mutation-based data structure, thereby not forcing callers to maintain a the underlying data structure. Classes are a powerful way to encapsulate data and routines that are mutation-based without exposing the details to callers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: a the

}
```

Because there is no need for a class when interacting with the Visual Studio Code API, Object Expressions are an ideal tool for this. They are also valuable for unit testing, when you want to stub out an interface with test routines in an ad-hoc manner.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ad-hoc -> ad hoc


### Avoid using type abbreviations to represent your domain

Although Type Abbreviations are convenient for giving a name to function signatures, they can be confusing when abbreviating other types. Consider this abbreviation:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type abbreviations capitalization is not consistent


Write `x :: l` with spaces around the `::` operator (`::` is an infix operator, hence surrounded by spaces) and `[1; 2; 3]` (`;` is a delimiter, hence followed by a space).

Always use at least one space between two distinct brace-like operators (e.g. leave a space between a `[` and a `{`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. -> for example

better to start a new sentence instead of putting the sentence in parentheses.


* Separate top-level function and class definitions with two blank lines.
* Method definitions inside a class are separated by a single blank line.
* Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. -> for example


### Use prefix syntax for generics (`Foo<T>`) in preference to postfix syntax (`T Foo`)

F# inherits both the postfix ML style of naming generic types, e.g. `int list` as well as the prefix .NET style, e.g. `list<int>`. You should prefer the .NET style, except for four specific types. For F# lists, use the postfix form: `int list` rather than `list<int>`. For options, use the postfix form: `int option` rather than `option<int>`. For arrays, use the syntactic name `int[]` rather than either `int array` or `array<int>`. For refs, use `int ref` rather than `ref<int>` or `Ref<int>`. For all other types, use the prefix form: `HashSet<int>`, `Dictionary<string,int>`, since this conforms to .NET standards. No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. -> for example

You should prefer seems an odd sentence. wondering if the four specific types should be on a list.


The following operators are defined in the F# standard library and should be used instead of defining equivalents. Using these operators is recommended as it tends to make code more readable and idiomatic. Developers with a background in OCaml or other functional programming language may be accustomed to different idioms, and this list summarizes the recommended F# operators.

```fsharp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

content inside code blocks doesn't get translated. Perhaps a table would be better?


### Use standard F# operators

The following operators are defined in the F# standard library and should be used instead of defining equivalents. Using these operators is recommended as it tends to make code more readable and idiomatic. Developers with a background in OCaml or other functional programming language may be accustomed to different idioms, and this list summarizes the recommended F# operators.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this list -> the following list (or table if you change to a table) and end with :

Consider making that last part a new sentence


The following articles describe guidelines for formatting F# code and topical guidance for features of the language and how they should be used.

This guidance has been formulated based on the use of F# in large codebases with a diverse group of programmers. We have found that this guidance will generally lead to successful use of F# and minimize frustrations when requirements for programs change over time.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove we


Tools are invaluable for working in large codebases, and there are ways to write F# code such that it can be used more effectively with tools. One example is making sure you don't overdo it with a point-free style of programming, so that intermediate values can be inspected with a debugger. Another example is using [XML documentation comments](../language-reference/xml-documentation.md) describing constructs such that tooltips in editors can display those comments at the call site. Always think about how your code will be read, navigated, debugged, and manipulated by other programmers with their tools.

## Read next
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the new templates call this section Next steps (can't access them right now). Or we usually call this See also.

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very useful stuff @cartermp I learned quite a bit reading it. Overall, this is a great set of guidance.

I had a number of suggestions for style, and a few locations where the guidance could be more actionable.


There are a few universal guidelines that apply to F# libraries, regardless of the intended audience for the library.

### Be familiar with the .NET Library Design Guidelines
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rephrase this to be active voice: "Learn the .NET ...", or "Know the .NET ..."


The .NET Library Design Guidelines provide general guidance regarding naming, designing classes and interfaces, member design (properties, methods, events, etc.) and more, and are a useful first point of reference for a variety of design guidance.

### Add XML documentation comments to your code
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A sentence or two explaining how the /// comments transform to XML documentation.

| Concrete types | PascalCase | Noun/ adjective | List, Double, Complex | Concrete types are structs, classes, enumerations, delegates, records, and unions. Though type names are traditionally lowercase in OCaml, F# has adopted the .NET naming scheme for types.
| DLLs | PascalCase | | Fabrikam.Core.dll | |
| Union tags | PascalCase | Noun | Some, Add, Success | Do not use a prefix in public APIs. Optionally use a prefix when internal, such as ```type Teams = TAlpha | TBeta | TDelta.``` |
| Event | PascalCase | Verb | ValueChanged | |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to add "ValueChanging" where appropriate?


#### Use interfaces to group related operations

We recommend you use interface types to represent a set of operations. This in preferred to other options, such as tuples of functions or records of functions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This in preferred => This is preferred

}
```

#### Use the “module of collection functions” pattern
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these patterns be defined, or will the intended audience be happy with an example and no formal definition? (Same comment on next header).

* Named and Optional arguments
* Interfaces and interface implementations

**Don't reach for these features first, but do judiciously apply them when they make sense:**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to expound on "when they make sense". Without some detail on when it makes sense, this isn't very helpful guidance.


The following articles describe guidelines for formatting F# code and topical guidance for features of the language and how they should be used.

This guidance has been formulated based on the use of F# in large codebases with a diverse group of programmers. This guidance will generally lead to successful use of F# and minimize frustrations when requirements for programs change over time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"will generally lead" => "generally leads"


## Five principles of good F# code

The following principles should be kept in mind any time you write F# code, especially in systems that will change over time. Every piece of guidance in further articles stems from these five points.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"You should keep the following principles in mind" would be a stronger start.


4. **Good F# code performs well without exposing mutation**

It's no secret that to write high-performance code, you must use mutation. It's how computers work, after all. Such code is often error-prone and difficult to get right. Exposing that mutation to callers should be avoided. A functional interface over a mutation-based implementation should be sought instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple instance of passive voice here:

Instead of "Exposing that mutation to callers should be avoided." try "Avoid exposing mutation to callers".

Instead of "A functional interface over a mutation-based implementation should be sought instead." try "Seek a functional interface over a mutation-based implementation."


5. **Good F# code is toolable**

Tools are invaluable for working in large codebases, and there are ways to write F# code such that it can be used more effectively with tools. One example is making sure you don't overdo it with a point-free style of programming, so that intermediate values can be inspected with a debugger. Another example is using [XML documentation comments](../language-reference/xml-documentation.md) describing constructs such that tooltips in editors can display those comments at the call site. Always think about how your code will be read, navigated, debugged, and manipulated by other programmers with their tools.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"..., and the there are ways" consider: "... you can write F# code..."

@selketjah
Copy link

selketjah commented May 8, 2018

I went through the style guide today. It is really great, there is a lot to like in it.
I have the following suggestions:

Language
Not everybody is native English, you might want to go over the text and see if some words can be replaced with more commonly known definitions.

For example:
* succinct (alternatives: concise, least amount of words possible)
* legible (alternatives: readable, easy to read)

Type annotations
different uses of spaces in the type annotations: all possibilities are present in the examples right now
let doStuff (x:int)
let doStuff (x : int)
let doStuff (x: int)

I don't know if it is being left to personal preference, but it might be a good idea to make a recommendation (or make it consistent in the style guide).

@AnthonyLloyd
Copy link

I've recently found the Rust documentation on error handling to be very good:

https://doc.rust-lang.org/book/second-edition/ch09-00-error-handling.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.