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

[Proposal] Pattern matching using Select TypeOf #541

Open
VBAndCs opened this issue Jul 8, 2020 · 34 comments
Open

[Proposal] Pattern matching using Select TypeOf #541

VBAndCs opened this issue Jul 8, 2020 · 34 comments

Comments

@VBAndCs
Copy link

VBAndCs commented Jul 8, 2020

I suggest this syntax for pattern matching in select statements:

Select TypeOf t
    Type x As String
         Console.WriteLine(x.Length)
    Type y As Integer
         Console.WriteLine(y + 1)
     Type Else
          Console.WriteLine(t.ToString())
End Select

Edit:

Which can be lowered to:

Select Case t.GetType
   Case GetType(String)
      Dim x = CStr(t)
      Console.WriteLine(x.Length)
   Case GetType(Integer)
      Dim y = CInt(t)
      Console.WriteLine(y+ 1)
   Case Else
      Console.WriteLine(t.ToString())
End Select

Based on #542 , I suggest this:

Select CType(t)
    Case x As String
         Console.WriteLine(x.Length)
    Case y As Integer
         Console.WriteLine(y + 1)
     Case Else
          Console.WriteLine(t.ToString())
End Select
@zspitz
Copy link

zspitz commented Jul 8, 2020

I have a few issues with this:

  1. What happens with assignable types? Presumably String should also match on IEnumerable(Of Char):

    Dim x As Object = "abcd"
    Select TypeOf x
        Type e As IEnumerable(Of Char)
            Console.WriteLine(e.Contains("d"C))
    End Select
    

    It would have to lower to something like this:

    If x IsNot Nothing AndAlso GetType(IEnumerable(Of Char)).IsAssignableFrom(x.GetType) Then
        Dim e = CType(x, IEnumerable(Of Char))
        Console.WriteLine(e.Contains("d"C))
    End If
    

    This is an echo of the confusion in the original TypeOf <x> Is <y> syntax -- TypeOf x Is SomeType sounds like an exact type check #277

  2. The general pattern matching idiom (AFAICT from C# and F#) is in the form, "Does this x match one of the following patterns?", some of which could be type patterns;, never "Does this x have a type-match in one of the following type patterns?". Limiting the whole Select to only type-matching seems inappropriate; especially since in both C# and F# you can mix and match type patterns along with other types of patterns.

  3. Using Type e is confusing, because what you're actually testing against is the type in the As clause; the variable is a side point once the test is successful. In that sense, the syntax proposed by @AdamSpeight2008 -- using Into for declaring these variables -- is far clearer: Case String Into s or even Type String Into s.

I think it would be helpful if you could clarify what benefits this syntax has over one of the type-checking patterns mentioned in #367.

@VBAndCs
Copy link
Author

VBAndCs commented Jul 8, 2020

Think of Type as a verb not a none:
Type x As String means make x of the type string. It is nearly equivalent to:
Dim x As String
I see this syntax more readable.

@zspitz
Copy link

zspitz commented Jul 8, 2020

I see this syntax more readable.

I think it's only because you associate declaring a variable with <Keyword> x As <TypeName>.

But consider -- the main point of the pattern is not the variable declaration, but the type check; the variable declaration follows from the type check -- "if x is type-compatible with T, only then declare a variable s of type T".

To emphasize this, how would you express a type-check without a variable declaration? With Into, the variable declaration can simply be made optional:

Select t
    Case String Into s
        Console.WriteLine($"A string of length {s.Length}")
    Case IEnumerable(Of Char)
        Console.WriteLine("Not a string, but an IEnumerable(Of Char)")
    Case Else
        Console.WriteLine("Some other object")
End Select

But if you insist on using some form of As, you must use a different variant syntax for this purpose:

Select t
    Type s As String
        Console.WriteLine($"A string of length {s.Length}")
    Type IEnumerable(Of Char)
        Console.WriteLine("Not a string, but an IEnumerable(Of Char)")
    Type Else
        Console.WriteLine("Some other object")
End Select

Also, Into already has similar usage within LINQ queries.

@VBAndCs
Copy link
Author

VBAndCs commented Jul 8, 2020

The meaning is obvious in both English and VB. Besides, we use exactly the same syntax today:
Catch x As OverFlowException
We don't use
Catch OverFlowException into x
and we can drop the variable
Catch OverFlowException
So, we can do the same in Select TypeOf:
Type IEnumerable(Of Char)
And if you prefere, the editor can auto add Is:
Type Is IEnumerable(Of Char)
as it does with Case < 10' that becomes: Case Is < 10`

@VBAndCs
Copy link
Author

VBAndCs commented Jul 8, 2020

Sorry:
catch (OverflowException) is allowed only in C# not VB, but I see it should be.

@AdamSpeight2008
Copy link
Contributor

I choose to use Into variable as it leverage on existing knowledge (LINQ) and style, doesn't require the creation of any new keywords. The equivalent english sentence would be roughly.
If TypeOf thisObject Is someThing Then put into this variable an not null instance of it

@VBAndCs
Copy link
Author

VBAndCs commented Jul 8, 2020

Same as my suggestion, and I see it better. I can drop type and use case, and it will give the same meaning:

Select TypeOf t
    Case x As String
         Console.WriteLine(x.Length)
    Case y As String
         Console.WriteLine(y + 1)
   Case Else
          Console.WriteLine(t.ToString())
End Select

VB uses the variable first in almost all places, and this should always be the case.

@AdamSpeight2008
Copy link
Contributor

VB uses the variable first in almost all places, and this should always be the case.

Especially in vb.net context is king.

Your syntax blur the meaning of a declaraion and a a type check., how would some learning the language understand the different semantic meaning?

@VBAndCs
Copy link
Author

VBAndCs commented Jul 8, 2020

As we did when we learned Catch e as Exception, which didn't confuse me at all.

@AdamSpeight2008
Copy link
Contributor

Since identifer As type = expression is predominantly semantically indicates a declaration.

When reading or understanding the code it is going artificially bias the as being more important (aka it has an higher order of precedence). thus changes the meaning from ((TypeOf e Is T ) As X) to (TypeOf e Is (T As X)).

In a catch statement Catch e as Exception, the e is e is a parameter in this case, not a declaration.

If we continue down the rabbit hole, I'd also expect to also to validly write, since I can currently do that when I write a declaration. TypeOf e Is t As New Fubar(sds)

@VBAndCs
Copy link
Author

VBAndCs commented Jul 8, 2020

Still see no issue. In fact, we need VB to allow declaring variables in place everywhere, such as:
Dim x = Integer.TryParse(inputVar, outVar As Integer)
So, this is how VB declare things, and we should have it everywhere possible.

@zspitz
Copy link

zspitz commented Jul 8, 2020

In fact, we need VB to allow declaring variables in place everywhere, such as:
Dim x = Integer.TryParse(inputVar, outVar As Integer)
So, this is how VB declare things, and we should have it everywhere possible.

Agreed. When the primary purpose of a construct is to declare a variable, <Identifier> As <TypeName> is appropriate.

But for pattern matching syntax, the primary purpose is not declaring a variable; it's matching against a pattern. The variable declaration is secondary to the pattern being matched.

And you still haven't clarified what benefit there is in Type x As String over Case <type pattern>.

@VBAndCs
Copy link
Author

VBAndCs commented Jul 8, 2020

This is exactly the primary purpose of type matching: casting type and assigning it to a var. Otherwise, we already match patterns for ever, but need further steps to assign them to vars.
I see no need to confuse us with new syntax, while the historical one is the perfect for the job.

@zspitz
Copy link

zspitz commented Jul 8, 2020

we already match patterns for ever

Absolutely, using Select Case:

Dim i As Integer
'...
Select Case i
    Case "a"
        Console.WriteLine("Matches the ""a"" pattern")
    Case > 5, 10 To 15
        Console.WriteLine("Matches the ""greater than 5, or between 10 and 15"" pattern)
End Select

I see no need to confuse us with new syntax,

such as Select Type instead of Select Case, or Type x As String instead of Case <type pattern> (whatever <type pattern> may be).

while the historical one is the perfect for the job.


This is exactly the primary purpose of type matching: casting type and assigning it to a var.

But this is not the primary purpose of pattern matching. Constraining pattern matching to only type matching, and to only capturing the entire subject of the Select, is an extremely limiting design decision, particularly since both C# and F# have no such limitations.

Admittedly, type-checking-and-variable-population is the most compelling use of pattern matching, but it is certainly not the only one.

Also, if the whole intention is just to satisfy this use case (of type matching and populating a variable), why should we need a new variable? #172 is a better choice -- redefine the type of the current variable/expression within the If TypeOf block.

@paul1956
Copy link

paul1956 commented Jul 9, 2020

If I look at several large databases of VB code I see 2 things over and over

Select Case True
  Case TypeOf Something Is SomeType
    Dim Y as SomeType = CType(Something , SomeType)      

and

Dim x As Integer =  nothing ' Requiring x to be initialized should not be neccessary if TryGet uses an <Out> attribute
Something.TryGet(x)

@zspitz
Copy link

zspitz commented Jul 9, 2020

@paul1956 The first code snippet is covered by #172, making the following code valid:

If TypeOf Something Is SomeType Then
    Something.MethodOfSomeType 'because Something is typed here as SomeType
Else
    'Something.MethodOfSomeType does not compile, because Something is not typed here as SomeType
End If

For the second snippet, which is preferable? This:

If Something.TryGet(Into x) Then
    Console.WriteLine(x)
Else
    'Console.WriteLine(x) does not compile, because x is uninitialized
End If

Or:

If Something.TryGet(Dim x As Integer) Then 'We can't use just Dim x, which everywhere else is equivalent to Dim x As Object
    Console.WriteLine(x)
Else
    'Console.WriteLine(x) does not compile, because x is uninitialized
End If

@paul1956
Copy link

paul1956 commented Jul 9, 2020

I prefer the second more VB like. How does C# handle the writeline(x) in the else I believe you can't access X and it is usually a throw? I would be fine if below worked without the warning when the parameter is declared . VB already support but from what I have seen it is ignored.

Dim X
Something.TryGet(x)

For the Select Case or If the concept of the type changing in the scope of the Block is very interesting but it may be confusing to someone reading the code.

@zspitz
Copy link

zspitz commented Jul 9, 2020

How does C# handle the writeline(x) in the else?

I think it's a compilation error, and I propose the same for VB.NET.

In the following code:

Dim x
Something.TryGet(x)

what should be the type of x? AFAICT x will currently be typed as Object. Unless you're suggesting the compiler look ahead to the TryGet to resolve the type of x?


it may be confusing to someone reading the code.

You always need some context to understand the types used by a piece of code:

' What's the type of i?
Dim i = Foo.Bar()

And in this case the precise type can be seen in the enclosing block.

@VBAndCs
Copy link
Author

VBAndCs commented Jul 9, 2020

Based on #542 , I suggest this :

Select CType(t)
    Case x As String
         Console.WriteLine(x.Length)
    Case y As Integer
         Console.WriteLine(y + 1)
     Case Else
          Console.WriteLine(t.ToString())
End Select

@zspitz
Copy link

zspitz commented Jul 9, 2020

Again, why do you think the start of the Select block needs to be anything other than Select t? At a minimum, you should want to have the flexibility of matching either against a type or against another Case-pattern:

Select t
    Case > 5
        Console.WriteLine(">5")
    Case y As Integer
         Console.WriteLine(y + 1)
    Case "abcd"
        Console.WriteLine("abcd")
    Case x As String
         Console.WriteLine(x.Length)
     Case Else
          Console.WriteLine(t.ToString())
End Select

which is something you can't do if you insist on Select CType(t) or Select TypeOf(t).

@VBAndCs
Copy link
Author

VBAndCs commented Jul 9, 2020

You can't compare with values unless you are certain of the Var type. Your example doesn't make any sense.

@zspitz
Copy link

zspitz commented Jul 9, 2020

Yes, you're right. The Case clauses need to be reordered. Edited.

@VBAndCs
Copy link
Author

VBAndCs commented Jul 9, 2020

This still messy. At least it will not work when Option Strict is On. I see no practical using worth concerning for mixed values and pattern match in the Select Case.

@zspitz
Copy link

zspitz commented Jul 9, 2020

It depends what the goal is, as I noted here. If the goal is generalized pattern matching, then there's no reason why this shouldn't be allowed even under Option Strict; both C# and F# -- strongly typed languages -- support generalized pattern matching in this way.

@AdamSpeight2008
Copy link
Contributor

It helpful if thing about what the lower form of the select - case with "patterns", will tend to be If ... Else ... End Ifs.
The TypeOf expr Is TSomething Into result is translate to call to helper method that does roughly the following.

Module HelperFunctions

  Public Function TypeOfIs(Of T As Class)( source As Object, ByRef output As T) As Boolean
    source = TryCast(source, T)
    Return source IsNot Nothing
  End Function

  Public Function TypeOfIs(Of T As Structure)( source As Object, ByRef output As T) As Boolean
    Dim temp = DirectCast(source, T?)
    output = temp.GetValueOrDefault()
    Return temp.HasValue
  End Function

End Function
Dim result As TSomething = Nothing
IF Helpers.TypeOfIs(Of TSomething)( expr, result ) Then

@AdamSpeight2008
Copy link
Contributor

AdamSpeight2008 commented Jul 9, 2020

The Into is not a general pattern, put is part the Type Is pattern. As in existing a type check Type expr Is TSomething when successful is after followed by a cast into that type. Dim result = DirectCast(expr, TSomething
It doesn't require Option Strict Off as the result's is implied by the preceding stated type.

@AdamSpeight2008
Copy link
Contributor

Partially implement the feature, before the troubles.
VBFeature_TypeOfInto
Still to do is remove the requirement of declaring the result variable before hand.

@VBAndCs
Copy link
Author

VBAndCs commented Jul 9, 2020

Still to do is remove the requirement of declaring the result variable before hand.

This is a deep trap in your design. What if I need to use an already existing var?
If you make it possible to use existing vars and declare non existing one, this will make typo errors declare unintended vars!
Above all, into is not a declaration keyword! Dim and Let are!
So, I see my #542 proposal better:
If CType(expr, result As String) Then

@AdamSpeight2008
Copy link
Contributor

The prototype is a work in progress, to get the general concept across and try out how it feels.

Already thought about that

If the identifier exists then
   If types are compatible Then
      use existing declaration
   Else
      Report an error  eg a incompatible type error
  End If
Else
  bring into scope an instance of the type with same name as the identifier.
End If

@paul1956
Copy link

@VBAndCs I want to get some attention but it is all over the place. I would like to model it after C# DeclarationPatternSyntax.

Select Case x
    Case TypeOf x is Something
        Dim y as Something = x
    Case TypeOf x is SomethingElse
        Dim z as SomethingElse = x
End Select

Becomes below with new feature

Select Case x
    Case TypeOf x is Something Dim y ' possible syntax
        ' y is a new variable of type Something and it's value = x
    Case TypeOf x is SomethingElse As z ' another syntax
        ' z is a new variable of type SomethingElse and it's value = x
End Select

If   TypeOf x is Something Dim y then
        ' y is a new variable of type Something and it's value = x
End if

Above might not be acceptable syntax but it is clear what it is doing.

@AdamSpeight2008
Copy link
Contributor

@paul1956 what about

Select Case x
    Case TypeOf x is Something Into y ' possible syntax
        ' y is a new variable of type Something and it's value = x
End Select

If TypeOf x is Something Into y Then
        ' y is a new variable of type Something and it's value = x
End If

@paul1956
Copy link

@AdamSpeight2008 I like it, it is clear to me what is happening much better then what I thought of. Also it would be very easy to do a CodeFix.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 18, 2020

Select Case x
    Case TypeOf x is Something Into y ' possible syntax
        ' y is a new variable of type Something and it's value = x
End Select

What is the purpose of Select Case x if you write the full condition in each case:
Case TypeOf x is Something Into y ' possible syntax
Isn't it an If statmenet but only missing Then?
If TypeOf x is Something Into y ' possible syntax

@VBAndCs
Copy link
Author

VBAndCs commented Oct 18, 2020

I think the perfect syntax can result from combining the Anthony's proposal with mine, so, we need to declare no new variables to deal with the target type. The Select TypeOf will be the indication to the compiler to do this trick:

Select TypeOf O
   Case Nothing
      Console.WriteLine("Nothing")
   Case String
      Console.WriteLine(O[0])
   Case Date
      Console.WriteLine(O.ToShortDateString( ))
End Select

Which can be lowered to:

If O is Nothing Then
   Console.WriteLine("Nothing")
ElseIf TypeOf O is String Then
   Dim O1 = CType(O, String)
   Console.WriteLine(O1[0])
ElseIf TypeOf O is Date Then
   Dim O2 = CType(O, String)
   Console.WriteLine(O2.ToShortDateString())
End Select

which can avoid any complications in Anthony's proposal.

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

No branches or pull requests

4 participants