-
Notifications
You must be signed in to change notification settings - Fork 21
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
Revamp treatment of optional values (esp records) #617
Comments
Overall I think this is a good idea. Two questions:
|
Did you know you can specify the type of the record currently in other ways like: Maybe another way to go about the options as optional thing would be with a keyword like
As it would mirror the way struct tuples and such look. |
@Rickasaurus I've seen that, and I think for the former, the problem is it may be far away from the actual For example, imagine you wanted to make a feature that automatically filled added For the latter variation, the problem is it comes after, which is also problematic for tooling support. |
@isaksky Makes sense! And w.r.t your tooling point, this does indeed make it easier. Eventually, we'll spec out what IntelliSense for F# needs to look like, and it's likely that we'll take a similar approach that Roslyn did where there are multiple providers. This would be a case for another IntelliSense provider and fits snugly with what I think to be a good approach to IntelliSense. |
Regarding inference of the correct record type, I often use the fully qualified field name for the first field as a type hint: let person1 = { Person.fullName = "Don Syme"; email = None } Regarding default values, this seems to go against the philosophy of F# in other places, where it avoids implicit default values like those in C#, which are more useful with mutation. let makePerson fullName = { Person.fullName = fullName; email = None; productivityFactor = 1.0 }
let person1 = { makePerson "Don Syme" with productivityFactor = 10.0 } Using a function, we can specify the mandatory values and this also gives you the type inference. |
@theprash A slight variation that is slightly more explicit would be:
So allow supplying default values in the record definition that will be used when nothing is specified for the field during initial construction. This way everything would (or could, depending on I think most people wouldn't be too shocked if |
One of the goals of this is to prevent needing to make record constructors private because of how brittle they are. As things stand now, you cannot expose them in libraries or non-trivial programs, because if you need to add a field, you just broke a ton of code. Even if none of that code cares about your field at all. Yes, you can make a factory type function that you expose instead, but then it is no longer first class, and people have to know about it. |
The "setting defaults" worries me. You add a new field to a record and won't know that you've forgotten to initialise it - it'll just be set to an empty list or 0.0 or whatever. |
I dont like the initial proposal
That said, the second variation you proposed is better. type InventoryRow =
{ name: string = "<unnamed>";
weight: float;
tags: string list = ["unlabeled"];
quantity: int = 1 } but that mean the values are harder to bind. what is going to be allowed?
|
F# already has this for classes: and InventoryRow()=
member val name : string = "<unnamed>" with get, set
member val weight : float = 0. with get, set
member val tags: string list = ["unlabeled"] with get, set Do you think this is a problematic feature of F#? If not, what is so different about this case (especially the second variation)? Keep in mind - if you want to be explicit, and have code that breaks when you add a new field, you already have that option, and will continue to have it even if this alternate syntax is added. @enricosada Also, we definitely would not want To make it easier on people just starting to read this, I'll update the main post with the other variation too. |
I think this suggestion could only be part of a more systematic look at how F# treats default values and optionality throughout a number of constructs, not just record types |
There's nothing stopping you from making a static member called Create that has optional arguments and returns a record. I personally would be abundantly careful of adding anything that puts more foot-guns on the desk. Defaults of any kind can be deceptively dangerous and are frequently a source of bugs. |
I think this would be really useful with anonymous records for Fable Typescript interop. When I have a typescript function function f(o : {a? : number, b? : string, c : boolean}) {
// ....
} then I can call it like this: f({a: 1, c: false}); // b is omitted and undefined Currently I have three options for the Fable bindings:
type FArgs = { a : float option; b : string option; c : bool }
[<Import("...")>]
let f(o:FArgs) = jsnative
f({a = Some 1.; b = None; c = false})
type FArgs =
| A of float
| B of string
| C of bool
let f(o:FArgs list) =
let o2 = keyValueList Fable.Core.CaseRules.LowerFirst o
// ...
let o =
f [ A 1.; C false | The union has the advantage that I don't need to name the optional members (which are often a lot), but makes the whole thing a bit awkward. I would really like to be able to use an Anonymous Record and omit the optional members.
IMO I would even restrict it to Anonymous Records only, because they are already a bit more loose than normal Records. |
The idea of default values seems very dangerous to me. Regarding collections, numbers and especially dates these values do not have a safe default because it depends so much on context. Defaults like DateTime.MinValue are never a great default. If someone does want all values to have a default, a default instance and the One properties I like the most about records is they're more like constructors than a bag of properties. When you add a property to a record, there is an error at each location it's constructed. You can't forget to add a property. I think the only case we should consider a default is for explicitly optional properties (like with Option<>) which default to One case where I think this would be valuable is when calling a function with a config style object, this is common as a js/ts style pattern but I think it's much more flexible than 12 overloads, especially since fsharp functions don't allow overloading. Consider the following HttpRequest style record where most of the properties are
|
I consider that a language design problem, since it leads to people not being able to this language feature if they do not inflict breaking changes on consumers of the code. I see that many F# people disagree, and love this part of F#. I find the trivial compile errors really problematic. One time when I needed to add a property to a record, I actually introduced bugs because I got so bored updating the 50+ callsites that didn't care about by new optional property that I stopped being careful.
I'm sure it is very contextual for many data types, but that weakness doesn't apply to the alternate syntax I suggested, where you can specify what the default is, similar to C#. |
I think standard objects already exist for when you have default values and better suit your described scenario where you don't want the code to turn red. There's nothing wrong with using objects when they fit the bill. I think if you have an optional field in your record you should take an option type for that field. You can even add a "getAge" function in the module for that record that returns |
Or you are actually protecting yourself from making mistakes. Getting bored for fixing your code is not a good excuse for introducing bugs. Referential integrity is a great thing, and records protect you from making changes that aren't compatible with your code. As a version between objects and records, you can always add a |
@voronoipotato That would only be a useful suggestion if I didn't care about any of the other features of F# records: like structural equality, creating new objects from existing ones, etc. Why should I have to give up those things just because I don't want my constructor calls breaking needlessly?
@abelbraaksma if having a property be defaulted would lead to errors, you would just not specify a default when creating the type definition for the record. And people don't need to pass whatever test you made up for what a 'good excuse' is. What matters is how these language constructs affects what people do in practice, not what you think they should have done. Humans are not perfectly conscientious and inexhaustible robots.
So I could do it some other way that involves writing a lot of boilerplate? I can do that in Java too. By that logic, there was no point adding string interpolation to F# either, right? After all, you could do it this other way... |
That's a fair point, for a long time, there was a strong sentiment not to implement string interpolation. And the net result is not the same as C# (mainly nested support, but also, F# adds mixin with sprintf style). I like the integrity with records, I wouldn't like throwing that away. You can break the integrity with I'm not saying your proposal is without merit, I can see some use cases, but I'd prefer an addition only if it doesn't break the integrity of records as it stands. Btw, I don't agree that using |
Another way to do this in F# - which isn't always a better fit than a constructor - is to have your different starting points represented as regular values like so type Bobble = { Ding: int; NewProp: int option }
let dingBobble = { Ding = 0; NewProp = None }
let code ding = { dingBobble with Ding = ding } |
@abelbraaksma What you say makes sense, my only objection would be that I think programmers can be trusted to just not provide a default value for a field if it the value can come as a surprise (or cause integrity problems as you say). Also, keep in mind this would only be for the alternate record construction syntax, so it wouldn't affect what is in use now. @NinoFloris I like that way, I think that is one of the best ways to do it today. My only issue with it is that it is another name, and another thing that will not necessarily exist for a record. So let's say I know a type name, For that strategy, if there were a way register a default type for a record, so that I could just write this, I think it would be a lot better (and would also solve my problem) : let code ding = { default<Bobble> with Ding = ding } |
I'd put it behind a static property on the type for easy discovery. |
I often have a static property for my record types called Empty or Initial, depending on what it represents, where it makes sense |
The term proto, or prototype might be a good word. |
The need for disambiguation isn't related to optional arguments. You can work around the ambiguity problem with Making constructor arguments automatically optional is a bad idea, but making them explicitly optional could work and has precedent with POCO constructors. To be consistent with these you would have: type MyRecord = {
X:int
?Y:int
?Z:int
}
let r1 = { X = 0; Z = Some 3}
let r3 = MyRecord(0, Z=3) There is a difference from POCO constructors, as you probably wouldn't have |
If we take over the other existing syntax, it would:
That is why it is not done that way (using the existing syntax) in this proposal.
That is not the intent here, this is from way before that became a thing. |
If optional fields must be explicitly declared as such, then I don't think so.
Mandatory fields would still break all your constructor sites. Optional fields wouldn't, but that's the whole point of optional fields :P |
This would be super useful when working with Fable and interfacing with Javascript components! |
The current record constructors are problematic, because if you add a field to it, everywhere that uses it is broken. Adding a field to a record is therefore often not backwards compatible, which is a big downside, and leads to some authors avoiding records altogether if they may be exposed externally.
This is a shame, because records and the record construction syntax are otherwise fantastic parts of F#.
Solution: Better record construction syntax
To solve this problem, I propose we add a new record construction syntax that is more clear, more tooling friendly, more concise, and less brittle. The current record constructors are problematic, because if you add a field
Summary:
%RecordName { ... }
syntax for constructing records (stolen from elixir)None
foroption
)null
for reference types does not countExamples:
For the alternative version:
Pros and Cons
The disadvantages of making this adjustment to F# are ...
Extra information
For point 2 of the summary, some implicit defaults to consider for types:
Somewhat related: Kotlin data classes.
Estimated cost (XS, S, M, L, XL, XXL): Unknown.
Related suggestions: (put links to related suggestions here)
Affidavit (please submit!)
Please tick this by placing a cross in the box:
Please tick all that apply:
The text was updated successfully, but these errors were encountered: