fslang-suggestions icon indicating copy to clipboard operation
fslang-suggestions copied to clipboard

if let expression

Open vasily-kirichenko opened this issue 6 years ago • 78 comments

https://github.com/MangelMaxime/Fulma/blob/1cbbc35e046007029e4b382d502cd9b301443a69/src/Fulma/Elements/Form/Textarea.fs#L144-L151

if Option.isSome opts.Id then yield Props.Id opts.Id.Value :> IHTMLProp
if Option.isSome opts.Value then yield Props.Value opts.Value.Value :> IHTMLProp
if Option.isSome opts.DefaultValue then yield Props.DefaultValue opts.DefaultValue.Value :> IHTMLProp
if Option.isSome opts.ValueOrDefault then
    yield Props.Ref <| (fun e -> if e |> isNull |> not && !!e?value <> !!opts.ValueOrDefault.Value then e?value <- !!opts.ValueOrDefault.Value) :> IHTMLProp
if Option.isSome opts.Placeholder then yield Props.Placeholder opts.Placeholder.Value :> IHTMLProp
if Option.isSome opts.OnChange then yield DOMAttr.OnChange opts.OnChange.Value :> IHTMLProp
if Option.isSome opts.Ref then yield Prop.Ref opts.Ref.Value :> IHTMLProp

I propose we add if let <pattern> then <expression> expression, so the above code would be rewritten as following:

if let (Some id) = opts.Id then yield Props.Id id :> IHTMLProp
if let (Some value) = opts.Value then yield Props.Value value :> IHTMLProp
if let (Some defValue) = opts.DefaultValue then yield Props.DefaultValue defValue :> IHTMLProp
if let (Some value) = opts.ValueOrDefault then
    yield Props.Ref <| (fun e -> if e |> isNull |> not && !!e?value <> !!value then e?value <- !!value) :> IHTMLProp
if let (Some p) = opts.Placeholder then yield Props.Placeholder p :> IHTMLProp
if let (Some onChange) = opts.OnChange then yield DOMAttr.OnChange onChange :> IHTMLProp
if let (Some r) = opts.Ref then yield Prop.Ref r :> IHTMLProp

It's inspired by the same Rust's feature, see https://doc.rust-lang.org/book/second-edition/ch06-03-if-let.html, and the same Swift's feature, see https://medium.com/@abhimuralidharan/if-let-if-var-guard-let-and-defer-statements-in-swift-4f87fe857eb6

The existing way of approaching this problem in F# is (see the first snippet, or match ... with Some x -> ... | None -> (), which is even more verbose, but safer).

Pros and Cons

The advantages of making this adjustment to F# are:

  • Shorter and safer code

The disadvantages of making this adjustment to F# are:

  • Nothing

Extra information

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

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • [x] This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • [x] I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • [x] 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:

  • [x] 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

vasily-kirichenko avatar Nov 07 '18 11:11 vasily-kirichenko

An interesting idea, but...

In closest current syntax, I think this:

if let (Some id) = opts.Id then yield Props.Id id :> IHTMLProp

becomes:

let (Some id) = opts.Id in yield Props.Id id :> IHTMLProp

Apart from this raising a warning, isn't the current syntax actually shorter? Assuming you mean that the let if syntax should not raise a warning for the missing cases and if let (Test x) = y is short for function Test x -> y | _ -> (), right?

And in your examples, there's no if .. then .. else or if .. then .. elif .. then .. else. Should those be supported? If not, the new expression could only ever return ().

If I understand you correctly, the syntax-gain isn't in less typing, but in removing warnings by creating a short-hand for a typical case of testing for a single DU-case and ignoring anything else.

abelbraaksma avatar Nov 07 '18 13:11 abelbraaksma

Oh wait, what I imply as "current" syntax, is a binding expression, and you mean to expand the if expression to create inner bindings similar to in, but with it returning something. Then still: what would be returned if the if test fails?

abelbraaksma avatar Nov 07 '18 13:11 abelbraaksma

if let is basically a single-case match without a required else or wildcard case. That means let behaves very differently when following if than it does elsewhere. But I suppose that difference is the perceived benefit of this suggestion.

I'm conflicted about whether this feels F#-ish or not.

FunctionalFirst avatar Nov 07 '18 22:11 FunctionalFirst

Then still: what would be returned if the if test fails?

I would assume unit, just like if without else returns today.

FunctionalFirst avatar Nov 07 '18 22:11 FunctionalFirst

if let is basically a single-case match without a required else or wildcard case.

@FunctionalFirst: But I assume then that the purpose is that you get the wildcard case for free, that is, it wouldn't throw an exception if the if-case isn't true. A single-case match without wildcard would throw, unless it's a single case DU, or is this syntax meant for single-case DU only? The examples speak otherwise (and there's already let-binding syntax for that anyway).

And, while we're at it, if this is accepted, it should probably support the whole range of patterns, right?

abelbraaksma avatar Nov 07 '18 23:11 abelbraaksma

An important distinction in the Rust docs:

Using if let means less typing, less indentation, and less boilerplate code. However, you lose the exhaustive checking that match enforces. Choosing between match and if let depends on what you’re doing in your particular situation and whether gaining conciseness is an appropriate trade-off for losing exhaustive checking.

Which makes sense, since the else is equivalent to a wildcard case.

As in Rust or Swift, the value bound with if let is only in scope for the if block.

Another interesting question would be extending this to elif:

type Foo =
    | Bar of int
    | Baz of int
    | Qux of int
    | Uhhh of string
    | Something of float

if let (Bar x) then
    printfn "Bar is %d" x
elif let (Baz x) then
    printfn "Baz is %d" x
else
    printfn "I don't care!"

The Swift use of if let is also interesting. In Swift, you can end up creating a pyramid of doom with if let. This is remediated by using guard let, which is a sort of inverse that you use to enforce invariants up front before processing data. This is done in the face of lots of optional data and only wanting to process something if all the data exists. I don't think if let would imply guard let in F#, but it's worth noting that the introduction of if let does introduce another pathway towards writing pyramids of doom.

cartermp avatar Nov 08 '18 00:11 cartermp

To clarify, this feature's intention is simplifying two-branches match expression, when the second pattern returns unit, so the following code

[ for i in 1..10 do
    match foo i with
    | Some x -> yield bar x
    | None -> () ]

would be written as

[ for i in 1..10 do
    if let (Some x) = i then yield bar x ]

So, if let ... then ... expression always returns unit and would look like an imperative feature, but it would work really nicely inside computation expression, as shown above.

It does not yield warning on incomplete match because it's the whole purpose of the feature: it evaluates the expression after then if, and only if a value satisfies the pattern.

About if ... elif ... elif... else ..., I strongly disagree it should be implemented because it would not add anything beyond normal match expression and would read much harder.

vasily-kirichenko avatar Nov 08 '18 07:11 vasily-kirichenko

If we implement this I'd expect else, elif and elif let to work (for consistency in the language)

matthid avatar Nov 08 '18 08:11 matthid

If indeed one of the main use cases is to use this in CEs, I would strongly suggest to include if let! into the proposal.

abelbraaksma avatar Nov 08 '18 12:11 abelbraaksma

@matthid I still disagree :) It provides a (definitely worse) alternative way to write matches, this should not be done.

vasily-kirichenko avatar Nov 08 '18 13:11 vasily-kirichenko

so the main purpose of this suggestion is to get rid of the | _ -> unit part of a match expression?

realvictorprm avatar Nov 08 '18 15:11 realvictorprm

Additional use cases would make this more compelling. @vasily-kirichenko's example can already be shortened slightly to if opts.Id.IsSome then yield opts.Id.Value :> IHTMLProp.

Option, like Nullable, has only two cases, and both of those types provide a property to check for the significant case (IsSome/HasValue). You could argue that this pattern should be used for all such types.

In other words, if let seems to solve a problem that's already solved by an established pattern. The benefit of the pattern is that it allows the author of the type to define canonical usage, which encourages consistency among its consumers.

FunctionalFirst avatar Nov 08 '18 16:11 FunctionalFirst

if opts.Id.IsSome then yield opts.Id.Value :> IHTMLProp is not safe.

vasily-kirichenko avatar Nov 08 '18 16:11 vasily-kirichenko

if opts.Id.IsSome then yield opts.Id.Value :> IHTMLProp is not safe.

@vasily-kirichenko How is .IsSome different than Option.isSome?

FunctionalFirst avatar Nov 08 '18 16:11 FunctionalFirst

@FunctionalFirst I mean, you can (and will) easily write

if opts.Id.IsNone then yield opts.Id.Value :> IHTMLProp

vasily-kirichenko avatar Nov 08 '18 16:11 vasily-kirichenko

And an IDE feature for converting between match and if let could be added :)

untitled

vasily-kirichenko avatar Nov 08 '18 17:11 vasily-kirichenko

It provides a (definitely worse) alternative way to write matches, this should not be done.

Indeed, but I feel like that is still "Ok" as you get 'punished' by having a bad and noisy syntax, if you missuse it. IMHO that is just enough to favor allowing 'else' and 'elif' and 'elif let'. This makes this a proper expression which is (again IMHO) more in line with F# philosophy.

matthid avatar Nov 08 '18 17:11 matthid

The whole feature does sort of fall under the principle of not offering multiple ways to do the same thing, but it's not like this would be the first feature that adds a different way to do things. And there is a precedent in other languages, plus it's in the spirit of F# to make things more pattern-based.

But elif feels like it would go a bit too far in the direction of offering a different way to do things. For example, if you covered all cases of a DU type with if let and elif let, what happens with else? A warning, like adding a discard case after a complete pattern match? But there's certainly an awkwardness with having if let but no ability to do elif let.

cartermp avatar Nov 08 '18 17:11 cartermp

@cartermp Yes and also the other way around

if someflag then
elif let ...
else

Why would that not be allowed?

matthid avatar Nov 08 '18 17:11 matthid

if you covered all cases of a DU type with if let and elif let, what happens with else?

You need to answer that anyway because it can be a single case union or a record. This question only disappears when else and elif is not allowed. But in that scenario it is no longer an expression (which would IMHO change this to a "don't do it"-feature)

matthid avatar Nov 08 '18 17:11 matthid

I would definitely prefer it if it were implemented as narrow as possible:

  • Only allow a single if, no else and no elif (because then you should use match).

I would also prefer to disallow let if for single case unions, for them you can already use

type T = | T of int
let myT = T 1
let (T i) = myT in printfn "%i" i

Edit: scratch the last part, it would still make sense for partial nested matches:

type TInner = A of int | B of int
type TOuter = | TOuter  of TInner 
let myT = TOuter  (A 1)
if let (TOuter (A i)) = myT then printfn "%i" i

0x53A avatar Nov 08 '18 18:11 0x53A

I personally think for symmetry elif should be supported. I would be deeply confused if it wasn't.

Also it allows things like this (which is good or bad depending on the observer)

if let (Some x)  = a then doSomething x
elif let (Some x) = b then doSomething x
else doNothing ()

mrange avatar Nov 12 '18 20:11 mrange

What about

if let (Some x) = a || let (Some x) = b then doSomething x else doNothing ()

?

0x53A avatar Nov 12 '18 21:11 0x53A

I personally think for symmetry elif should be supported. I would be deeply confused if it wasn't.

Also it allows things like this (which is good or bad depending on the observer)

if let (Some x)  = a then doSomething x
elif let (Some x) = b then doSomething x
else doNothing ()

That seems like a wordier way to write this (existing syntax)—

match a, b with
| Some x, _ | _, Some x -> doSomething x
| _ -> doNothing()

FunctionalFirst avatar Nov 13 '18 17:11 FunctionalFirst

That's my primary concern with this feature. if let in isolation has prior art to look at and makes sense in isolation. But making it more complete leads us right down the path of creating an alternative to match expressions.

cartermp avatar Nov 13 '18 17:11 cartermp

I personally think for symmetry elif should be supported. I would be deeply confused if it wasn't. Also it allows things like this (which is good or bad depending on the observer)

if let (Some x)  = a then doSomething x
elif let (Some x) = b then doSomething x
else doNothing ()

That seems like a wordier way to write this (existing syntax)—

match a, b with
| Some x, _ | _, Some x -> doSomething x
| _ -> doNothing()

But if it's just

if let (Some x)  = a then doSomething x
elif let (Some x) = b then doSomething x

Then it's saves the default case... which is ~only thing this feature saves on first-place. So vote for elif let for consistency if this get accepted.

Risord avatar Nov 14 '18 00:11 Risord

I'm not sure if I like or dislike the feature. Like others have said I think it could feel confusing if elif was not supported.

I'm also not sure this is the right place for code suggestions (I'm new here :P). In this particular case however, wouldn't the clunkyness go away if instead trying to pull every value out of the Option, one were to put everything into Option and remove all the boolean logic?

let f g x = (g x) :> IHTMLProp
let htmlProperties = 
     List.choose id [ yield classes |> Some
                      yield Props.Disabled opts.Disabled :> IHTMLProp |> Some
                      yield Props.ReadOnly opts.IsReadOnly :> IHTMLProp |> Some
                      yield Option.map (f Props.Id) opts.Id
                      yield Option.map (f Props.Value) opts.Value 
                      // And so on.. ]
in textarea htmlProperties children

ghost avatar Nov 14 '18 17:11 ghost

The proposed syntax is surprising and unexpected, and the value it adds is honestly also unclear to me.

type TInner = A of int | B of int
type TOuter = | TOuter  of TInner 
let myT = TOuter  (A 1)
//currently works great
myT |> (fun (TOuter (A i)) -> printfn "%i" i)
//pointfree also works
(fun (TOuter (A i)) -> printfn "%i" i) myT
//less parentheses also works
match myT with TOuter (A i) -> printfn "%i" i
//this is actually longer than the traditional match because 
//like the fun it has parentheses that can't be omitted due to ambiguity
if let (TOuter (A i)) = myT then printfn "%i" i

Do we really absolutely need an nth way to destructure / match?

you can let bind, you can computation expression let bind, you can use a bind function, you can match, you can function match, you can create a function that does all this in one step.

voronoipotato avatar Nov 17 '18 02:11 voronoipotato

@voronoipotato have you read the proposal? seen the example from a Fable app? questions left?

vasily-kirichenko avatar Nov 17 '18 06:11 vasily-kirichenko

I understand a little better having read some of the comments. It was non-obvious to me that not specifying the none case causes an exception. I haven't ever run into "not filled out the none case" as a problem. if let definitely was not obvious to me what should happen. I get that these other languages do it that way, but pulling idioms from other languages IMHO should always be done carefully as it is a good way to alienate both newcomers and people who love the language as it is. What are the odds that someone coming to F# has used rust/swift extensively given that I don't even think they interop nicely. It's VERY tempting for us to be implementing a thing in every syntax of every adjacent language, but in the end it's more valuable to be true to ourselves. We have the haskellers who want to mimic haskell's syntax and the C#'ers who want to mimic C#'s syntax, and the OCaml people, and the rust people etc. If we do that we're just risking creating an environment where people fight over what syntax is good, and each person just locks down on the version they're familiar with.

Additionally what happens when there's more than one case are we going to do "If then elseif" and this leads to my next concern. It's also entirely new syntax in an area that in my opinion is already very cluttered. See fun, function, match, let with destructuring, not to mention are we going to add if let to computation expressions. In terms of approaches there's several ways you can already go, you can create a function, you can match, you can use a function based match, a lambda destructuring. There are several existing approaches the authors of that code could have used to fix the line bloat.

If we're going to make any change (and I'm not confident we should), I would recommend extending match/fun/function to support automatic wildcard matching for "unit", since the return type must always be unit anyway.

// we could modify match so that when the return is unit
// it won't throw an exception for unmatched cases. 
// it would be essentially syntactic sugar for wildcard to unit since it must always be unit.
match t with Some(t) -> yield t
// and lambdas
fun (Some t) -> yield t
//also
fun (Some t) -> printfn "%s" t 
//functions too...
function Some t -> printfn "%s" t

I think using matches or lamdbas is much more explicit that it's an action rather than an assignment and is more in line with how F# currently reads.

voronoipotato avatar Nov 17 '18 08:11 voronoipotato