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

if let expression #705

Open
vasily-kirichenko opened this Issue Nov 7, 2018 · 39 comments

Comments

Projects
None yet
@vasily-kirichenko

vasily-kirichenko commented Nov 7, 2018

if let expression

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:

  • 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
@abelbraaksma

This comment has been minimized.

abelbraaksma commented Nov 7, 2018

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

This comment has been minimized.

abelbraaksma commented Nov 7, 2018

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?

@FunctionalFirst

This comment has been minimized.

FunctionalFirst commented Nov 7, 2018

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

This comment has been minimized.

FunctionalFirst commented Nov 7, 2018

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

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

@abelbraaksma

This comment has been minimized.

abelbraaksma commented Nov 7, 2018

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?

@cartermp

This comment has been minimized.

Member

cartermp commented Nov 8, 2018

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.

@vasily-kirichenko

This comment has been minimized.

vasily-kirichenko commented Nov 8, 2018

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.

@matthid

This comment has been minimized.

matthid commented Nov 8, 2018

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

@abelbraaksma

This comment has been minimized.

abelbraaksma commented Nov 8, 2018

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.

@vasily-kirichenko

This comment has been minimized.

vasily-kirichenko commented Nov 8, 2018

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

@realvictorprm

This comment has been minimized.

Member

realvictorprm commented Nov 8, 2018

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

@FunctionalFirst

This comment has been minimized.

FunctionalFirst commented Nov 8, 2018

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.

@vasily-kirichenko

This comment has been minimized.

vasily-kirichenko commented Nov 8, 2018

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

@FunctionalFirst

This comment has been minimized.

FunctionalFirst commented Nov 8, 2018

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

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

@vasily-kirichenko

This comment has been minimized.

vasily-kirichenko commented Nov 8, 2018

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

if opts.Id.IsNone then yield opts.Id.Value :> IHTMLProp
@vasily-kirichenko

This comment has been minimized.

vasily-kirichenko commented Nov 8, 2018

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

untitled

@matthid

This comment has been minimized.

matthid commented Nov 8, 2018

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.

@cartermp

This comment has been minimized.

Member

cartermp commented Nov 8, 2018

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.

@matthid

This comment has been minimized.

matthid commented Nov 8, 2018

@cartermp Yes and also the other way around

if someflag then
elif let ...
else

Why would that not be allowed?

@matthid

This comment has been minimized.

matthid commented Nov 8, 2018

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)

@0x53A

This comment has been minimized.

Contributor

0x53A commented Nov 8, 2018

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
@mrange

This comment has been minimized.

mrange commented Nov 12, 2018

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 ()
@0x53A

This comment has been minimized.

Contributor

0x53A commented Nov 12, 2018

What about

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

?

@FunctionalFirst

This comment has been minimized.

FunctionalFirst commented Nov 13, 2018

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()
@cartermp

This comment has been minimized.

Member

cartermp commented Nov 13, 2018

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.

@Risord

This comment has been minimized.

Risord commented Nov 14, 2018

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.

@erbaman

This comment has been minimized.

erbaman commented Nov 14, 2018

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 lift 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
@voronoipotato

This comment has been minimized.

voronoipotato commented Nov 17, 2018

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.

@vasily-kirichenko

This comment has been minimized.

vasily-kirichenko commented Nov 17, 2018

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

@voronoipotato

This comment has been minimized.

voronoipotato commented Nov 17, 2018

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.

@vasily-kirichenko

This comment has been minimized.

vasily-kirichenko commented Nov 17, 2018

Look, I created this suggestion when I saw that Fable code, let me repeat it here:

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

What's wrong with it?

  • The most important pitfall: It's unsafe, because you can easily write if Option.isNone opts.Ref then yield Prop.Ref opts.Ref.Value :> IHTMLProp or any code which test a value with isSome, but access another value in then... expression. It's just too error prone.
  • Far less important, but still: It's more verbose that it should be.

You say there are already too many ways to destruct values. Yes, but there is no one suitable for this particular case. Personally, I'd write it as match opts.Id with Some id -> yield Props.Id id :> IHTMLProp | None -> () (or | _ -> ()), but it's just boilerplate-full and it's clear that we do someting imperative (kind of) here anyway: we yield if it's Some, and we do nothing, if it's None. Computation expressions blur the line between pure and side effectfull code though, so if F# does not have CEs, I'd not propose this at all. But inside CEs this seems to works very nicely.

I ask you to write a variant of the following code, which will 1. Safe 2. Reasonably short. You can define helper functions if you need (you cannot define custom CE operations, because the code must work inside list/array/seq expressions).

Thanks!

@et1975

This comment has been minimized.

et1975 commented Nov 17, 2018

Here's another example where it would be useful:

while Option.isSome nextMsg do 
    let msg = nextMsg.Value
@Nhowka

This comment has been minimized.

Nhowka commented Nov 17, 2018

As using if could hint in having symmetry in supporting elif and else, I propose instead that we use when without needing the let.

when (Some x) = b then doSomething x

That would make it more intuitive that it is something unity. Or even instead of then we could use do:

when (Some id) = opts.Id do yield Props.Id id :> IHTMLProp
@erbaman

This comment has been minimized.

erbaman commented Nov 19, 2018

@vasily-kirichenko

I ask you to write a variant of the following code, which will 1. Safe 2. Reasonably short. You can define helper functions if you need (you cannot define custom CE operations, because the code must work inside list/array/seq expressions).

What about my suggestion ?

In addition to that I guess you could create a CE like "optionList" and have the wrapping in Some be implicit with yield.

@voronoipotato

This comment has been minimized.

voronoipotato commented Nov 20, 2018

This is my first crack at the problem. I realized that this resembled nested mapping of lists , except with an option and a list. I used choose here to gobble up our None since that's what its for. This lets us treat all our options as the things inside the options and so the code is in my opinion much easier to read. If you want me to write this with yield I'll give it a shot but I'm less experienced with yield so it might take me a little longer.

let first = Some "test"
let second = None
let third = Some "2"
let fourth = Some "3"
let fifth = None
let sixth = Some "hmm"
//Example setup... 

//Heres where you would do your (a b :> IHTMLProp) or whatever
//Use currying to permit any number of arguments, just put your option in the last position
let f (a: string) (b: string) =  (b + a) 

//the syntax of this still allows for inline declarations etc.
//i made the argument order this way to preserve the look and feel of the original code
//also it visually tracks like pipes
//choose is map but gobbles up the nones.
let l =  List.choose (fun (x,g) -> Option.map g x ) [
  first , f "ing"
  second , f "no"
  third , f "yay"
  fourth, f "woah"
  fifth, fun fifth -> fifth + "yes!"
  sixth, f "6" 
  ] 
@voronoipotato

This comment has been minimized.

voronoipotato commented Nov 20, 2018

let first = Some "test"
let second = None
let third = Some "2"
let fourth = Some "3"
let fifth = None
let sixth = Some "hmm"
//Example setup... 

//goofy goober option, make an Option Pipe operator.
//it has two '>' because you're really forcing it in there 😄 
let inline (|>>) x y = Option.map y x
//Heres where you would do your (a b :> IHTMLProp) or whatever
let f (a: string) (b: string) =  (b + a) 
let g = f "got it"
//the syntax of this still allows for inline declarations etc.
//i did it this way to preserve the look and feel of the original code
//choose gobbles up the nones. Map lets us map our function over options
let test = 
  List.choose (id) [
    yield first |>> (f "test")
    yield second |> Option.map (fun _ -> "don't care")
    yield Option.map g third
    yield Option.map g fourth
    ] 
@dsyme

This comment has been minimized.

Collaborator

dsyme commented Nov 21, 2018

Looking at the original sample,

match opts.Id with Some id -> yield Props.Id id :> IHTMLProp | _ -> ()

is only a handful of characters longer than this (71 to 63).

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

Is it really worth it for 8 characters?

@voronoipotato

This comment has been minimized.

voronoipotato commented Nov 21, 2018

I'm still proud of my "fat-pipes" operator 😄

@Richiban

This comment has been minimized.

Richiban commented Dec 4, 2018

I'm assuming that the only real reason the if statement is even in F# is because (a) it's expected for users coming from other languages and (b) it allows a 'statement' form where the else branch may be omitted.

Otherwise, the two forms seem isomorphic.

let b = true

match b with true -> printfn "Yes" | false -> ()

// vs

if b then printfn "Yes"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment