Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow access to record type constructors in F# #722

Open
charlesroddie opened this Issue Feb 27, 2019 · 14 comments

Comments

Projects
None yet
9 participants
@charlesroddie
Copy link

charlesroddie commented Feb 27, 2019

F# has concise syntax for defining records:

type PensionData =
    { Name:string; ProbableNumberOfYearsUntilRetirement:int }

Creation of records in C# is easy, using the constructor:

PensionData r = PensionData("Adam",10)

This is convenient as to enter the data you can just type the class name, a bracket, and intellisense will tell you what the fields should be.

Creating records in F# is clunky, because the constructor is not accessible:

let r = { Name = "Adam"; ProbableNumberOfYearsUntilRetirement = 10 }

The disadvantages are:

  • Verbose syntax compared to C#, with all field names needing to be written out.
  • Poor intellisense support. Without the binding to r it would be hard to determine the type of the record. In order to enter the record, you have to know the field names, and intellisense does not assist.
  • Activation of an unreliable type-inference system which relies on guesswork: record type inference guesses types based on field names entered. This requires an annotation to solve, which is extra verbosity.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S

Related suggestions:

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this

@charlesroddie charlesroddie changed the title Allow access to record type primary constructor in F# Allow access to record type constructors in F# Feb 27, 2019

@abelbraaksma

This comment has been minimized.

Copy link

abelbraaksma commented Feb 28, 2019

For a contrasting opinion (though I'm not against the proposal per se), I've just refactored hundred or so POCO's into records, they used C# class style in F#, so that I could use the verbose syntax. Simple reason? Readability. I went nuts over each time trying to find out what each argument in the constructor was, jumping back and forth to the definition.

After refactoring was done, code was easier to follow and reason about, and I felt sorry for all C# programmers (including myself) not having such clear syntax.

Also, the recently much improved inference doesn't give me any serious trouble.

That said, esp for records with one or two fields, I can see the benefit of having two ways of doing things.

@davidglassborow

This comment has been minimized.

Copy link

davidglassborow commented Feb 28, 2019

https://sharplab.io/#v2:DYLgZgzgNAJiDUAfALgTwA4FMAEAFTAThAPYB22AvNgN7YCCA5jiNgJanLYC+QA=

Doesn't look like the constructor is hidden by attribute or something, so not sure how F# ignores the constructor

@cartermp

This comment has been minimized.

Copy link
Member

cartermp commented Feb 28, 2019

Just a note on IntelliSense, there are some things it helps with today:

  • By being after a { we know we're inside of a record
  • Typing anything in between the brackets will filter completion based on record label names if it can

image

But if it's not the first character the preselected item will be different:

image

@charlesroddie

This comment has been minimized.

Copy link
Author

charlesroddie commented Mar 2, 2019

Is this actually a language suggestion? This is not explicitly prohibited in the language specification as far as I can see. If someone figures out what is preventing access to the constructor and submits a PR, will that be OK?

@voronoipotato

This comment has been minimized.

Copy link

voronoipotato commented Mar 3, 2019

I personally don't see anything wrong with constructing records with
record(a,b), it's basically just a shorthand for a create function. Perhaps we could have the developer be able to write a create function that returns the record. I've thought about how this would be nice for DU's where we have some private DU to prevent the creation of it without our create function, however usually I want a create that returns an Option rather than a simple DU. The reason I want my type to be private is because I don't want to have other developers creating that DU without following the constraints I have on that DU.

@davidglassborow

This comment has been minimized.

Copy link

davidglassborow commented Mar 4, 2019

As a side note, rather than a private DU, I just go for the approach of rebinding the constructor, although I never see others do it, so I must be missing something

type IntLargerThanZero = IntLargerThanZero of value:int
let IntLargerThanZero x = if x > 0 then IntLargerThanZero x |> Some else None
@theprash

This comment has been minimized.

Copy link

theprash commented Mar 4, 2019

To me, the existing verbose record construction fits in with the general F# theme of being more explicit than implicit where it can affect safety, as in the ability to refactor without changing behaviour. Currently the order of record fields has almost no effect on behaviour and it's safe to re-order them as a refactoring. But suppose you had a record with multiple fields of the same type:

type PensionData =
    { Name : string
      ProbableNumberOfYearsUntilRetirement : int
      YearsWorked : int }

If we decide to swap the position of the last two fields in the definition then we change the meaning of any usage of a constructor that relies on the order (e.g. PensionData("Name", 10, 30)), without creating any compiler errors.

I would consider this to be the biggest disadvantage because I feel that anything that can subtly break your runtime is an order of magnitude more inconvenient in the long run than typing some more identifiers.

@voronoipotato

This comment has been minimized.

Copy link

voronoipotato commented Mar 4, 2019

@davidglassborow well I mostly didn't do that because I didn't even know that you COULD do that. Given that you can just rebind the constructor, maybe if we have a default constructor it should simply be alphabetical. If you want a specific order for your constructor you can rebind them as @davidglassborow shows. This way we can shuffle the order of arguments in our traditional way of doing records as we please without breaking the build. If someone wants that specific order represented in the auto-constructor they can rebind it, and perhaps we can include that in documentation.

@BentTranberg

This comment has been minimized.

Copy link

BentTranberg commented Mar 8, 2019

I agree with @theprash, and I can also imagine situations where things can start to go wrong because the compiler is no longer as strict. And this will affect my coding even if I do not want to use this new feature. I question whether it is correct that this is not a breaking change to the F# language design. Let's say I want to change my class into a record. Then I have to watch out for any class constructors mistakenly being left as this new kind of record constructor, which is not what I want. I want the compiler to continue to alert me of problems in this case, just as it will do whenever I add or remove a case to a DU. Let's not spoil that.

Besides, I don't see the point. It's very easy to do this.

let pensionData name probableNumberOfYearsUntilRetirement =
    { Name = name; ProbableNumberOfYearsUntilRetirement = probableNumberOfYearsUntilRetirement }

let pd = pensionData "name" 150
@charlesroddie

This comment has been minimized.

Copy link
Author

charlesroddie commented Mar 8, 2019

@theprash To me, the existing verbose record construction fits in with the general F# theme of being more explicit than implicit where it can affect safety

Debatable. The intellisense guesswork on records means that it often mistakes one record type for another which has the same or similar field names. Record-specific construction is explicit about fields but not explicit about the type. (NB classes can be explicit about fields too via named arguments.)

@BentTranberg I question whether it is correct that this is not a breaking change to the F# language design... this new kind of record constructor

This is not a breaking change. This is not debatable. No code currently compiling breaks when the constructor is unhidden. This constructor also is not new.

It's very easy to do this...

We have a static Create method defined on every record and it would be nice to get rid of these lines of code which are duplicating functionality that is currently exposed to all .Net code except F#.

@cartermp

This comment has been minimized.

Copy link
Member

cartermp commented Mar 8, 2019

Also worth noting that the only sane way to construct records and ensure they work without issue with other lang constructs and refactorings is to apply even more verbosity: https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-records

@dsyme

This comment has been minimized.

Copy link
Collaborator

dsyme commented Mar 9, 2019

Just to note that one specific reason to allow this is that it allows using the record constructor as a first class value.

@7sharp9

This comment has been minimized.

Copy link
Member

7sharp9 commented Mar 9, 2019

@voronoipotato

This comment has been minimized.

Copy link

voronoipotato commented Mar 25, 2019

This may end up becoming a separate but somewhat related RFC but OCaml has field and label punning for record construction. This means if you have a variable that is the same name as the record label you can just put it in there.

let create_host_info ~hostname ~os_name ~cpu_arch ~os_release =
    { os_name; cpu_arch; os_release;
      hostname = String.lowercase hostname;
      timestamp = Time.now () };;

versus

 let create_host_info
    ~hostname:hostname ~os_name:os_name
    ~cpu_arch:cpu_arch ~os_release:os_release =
    { os_name = os_name;
      cpu_arch = cpu_arch;
      os_release = os_release;
      hostname = String.lowercase hostname;
      timestamp = Time.now () };;

obviously the ~labels help with this specifically because they address @theprash 's concern directly. Is there any RFC for labeled arguments?

(see https://v1.realworldocaml.org/v1/en/html/records.html )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.