Method overloading and pattern matching

[QsROg8320]

Hi, guys! What do you think about combination of pattern matching and method overloading?
Some example:

public class Shape 
{
	public double X {get;set;}
	public double Y {get;set;}
}	
public class Rectangle:Shape
{
	public double Width {get;set;}
	public double Height {get;set;}
}
public class Circle:Shape
{
	public double Radius {get;set;
}					
public class SomeClass
{
	
	//execute if s is Circle and s.Radius >10
	public void DoSomething(Shape s) when s is Circle => s.Radius>10
	{
		
	}
	//execute if s is Rectangle and s.Width >10
	public void DoSomething(Shape s) when s is Rectangle => s.Width>10
	{
		
	}
	
}

[bondsbw]

Duplicate of #1468.

Soon these (or similar syntax) will give you the same result:

void DoSomething(Shape s) 
{
    if (s is Circle { Radius > 10 })
    {
        ...
    }
    else if (s is Rectangle { Width > 10 })
    {
        ...
    }
}

void DoSomething(Shape s) => s match 
    {
        Circle c when c.Radius > 10 => ...,
        Rectangle r when r.Width > 10 => ...
    };

Where your proposal can benefit is playing well with partial classes and inheritance. Those alone are probably not compelling enough to warrant the work required to deliver the feature.

[bondsbw]

(Actually my second example might not work given the void return.)

[QsROg8320]

I think my proposal is better.
Your first example is look good for two cases, but if we have 10 cases your code will be unreadable.

void DoSomething(Shape s) 
{
    if (s is Circle { Radius < 5 })
    {
        ...
    }
    else if (s is Rectangle { Width > 10 })
    {
        ...
    }
    else if (s is Triangle{ Width > 10 })
    {
        ...
    }
    else if (s is Circle{ Radius> 10 })
    {
        ...
    }
    else if (s is Square{ X > 100 })
    {
        ...
    }
    else if (s is Triangle{ Width > 10 })
    {
        ...
    }    
     else if (s is Square{ Y> 10 })  
    {
        ...
    } 
     ...
}

Your second example is not good for big body of method. We have to write extra methods

void DoSomething(Shape s) => s switch
{
    Circle c when c.Radius > 10 => MethodCircleWithRadiusMore10(c),
    Rectangle r when r.Width > 10 => MethodRectangleWithRadiusMore10(r)
};
void MethodCircleWithRadiusMore10( Circle  c)
{
   //thousands of lines of code here
}
void MethodRectangleWithRadiusMore10( Rectangle r)
{
   //thousands of lines of code here
}

[bondsbw]

A switch block can simplify multiple if blocks.

[bondsbw]

Your proposal doesn't address precedence. You have two tests for Square, one checks X > 100 and the other Y > 10. Which is called if both apply? It is clear with either if or switch.

[QsROg8320]

I think it has to be compilation error, something about ambiguity.

[bondsbw]

This proposal doesn't make sense to decide at compile time. The compiler can't know runtime property values.

And the compiler either doesn't know the subclass of Shape being passed as the argument, or it does and a standard overload would be sufficient.

[QsROg8320]

I think compiler can find ambiguity if we use constants like in my examples Circle => s.Radius>10 Circle => s.Radius>12 in another case we can choose just any appropriate method or throw runtime error. I don't know precise answer, but I think that is solvable problem.

[orthoxerox]

@bondsbw @Malanin this proposal looks like it's inspired by ML-like languages, but in ML-like languages the order of function declaration matters (that's why you need let rec and have to reorder F# source files). In C# this has always not been the case, you can reorder types and methods without affecting the semantics of the program. This means that unfortunately, this proposal won't fly.

However, switch expressions and recursive patterns will let you write quite concise code structurally similar to ML:

public void DoSomething(Shape s) => s switch {
    Circle c when c.Radius > 10 => ... ,
    Rectangle r when r.Width > 10 => ... ,
    ...
};

This makes me wonder if we need shorthand syntax for switch expressions as method bodies. This final semicolon looks so out of place.

[DavidArno]

I have two problems with this proposal; one minor and one major.

Firstly, the minor one: your choice of syntax. If I see

DoSomething(Shape s) when s is Circle => s.Radius>10

I see an expression bodied method that will execute if s is Circle and will return the boolean result of s.Radius>10. But your method is a void with a method body. So I think your syntax confusing. The following would make more sense to me:

public void DoSomething(Shape s) when s is Circle && s.Radius>10
{

}

But this point is fairly moot compared with my main concern: encouraging weak cohesion, especillay when we take into account your comment "Your second example is not good for big body of method. We have to write extra methods". If I have a method containing a switch statement that is thousands of lines long, the absolute last thing I'd want is that switch statement replaced with a bunch of methods, each a thousand lines long, where each method contains part of the story as to which one would get executed in a particular circumstance.

Having a small public method that does the switch, and a bunch of private methods that handle each case is exactly what I'd want. So you example code is the correct way to do things in my view (save for the "thousands of lines of code here" part; that's inexcusable always):

void DoSomething(Shape s) => s switch
{
    Circle c when c.Radius > 10 => DoSomethingWithCircleWithRadiusGreaterThan10(c),
    Rectangle r when r.Width > 10 => DoSomethingWithRectWithWidthGreaterThan10(r)
};
void DoSomethingWithCircleWithRadiusGreaterThan10(Circle  c)
{
   //a few lines of code here
}
void DoSomethingWithRectWithWidthGreaterThan10(Rectangle r)
{
   //a few lines of code here
}

Though of course we should also remember that the switch expression will not work with void, so we'd have to use the existing clunky switch statement here instead (or put a sensible return type in there).

[QsROg8320]

@DavidArno, I agree with your first statement public void DoSomething(Shape s) when s is Circle && s.Radius>10 is beter syntax then mine. But I don't understand why do you prefer write more code
First Example:

void DoSomething(Shape s) => s switch
{
    Circle c when c.Radius > 10 => DoSomethingWithCircleWithRadiusGreaterThan10(c),
    Rectangle r when r.Width > 10 => DoSomethingWithRectWithWidthGreaterThan10(r)
};
void DoSomethingWithCircleWithRadiusGreaterThan10(Circle  c)
{
   //a few lines of code here
}
void DoSomethingWithRectWithWidthGreaterThan10(Rectangle r)
{
   //a few lines of code here
}

Second Example:

public void DoSomething(Shape s) when s is Circle && s.Radius>10
{
//a few lines of code here
}
 public void DoSomething(Shape s) when s is Rectangle && s.Width > 10
{
//a few lines of code here
}

Just compare this two examples. My proposal make code cleaner, shorter and more readable.

[svick]

@Malanin I think your proposal will make code harder to understand and navigate.

It will be harder to understand, because it won't be clear which version of the method will be executed. To some degree, that's already the case with overloads, but you're making that much worse.

It will be harder to navigate, because today I can easily use a gesture in my IDE to find which method or overload will be called by a line of code. But you proposal will make that navigation much harder: which version of the method should the IDE show, if any one of them can be executed at runtime?

[QsROg8320]

@svick, I don't see a problem with IDE. If we use interface instead of class ( when we use Dependency Injection), we also can't navigate on needed method, but visual studio show list of variants and we chose one we want.

[svick]

@Malanin You can at least navigate to the interface method declaration, which can give you some useful information.

Also: how will your proposal deal with XML documentation comments and attributes (including those on method parameters)?

[QsROg8320]

All methods have a single purpose and there is no need to write difference description for each method, methods has the same parameters. I think IDE can choose any xml comment.

[QsROg8320]

@HaloFour, It is not big problem. We can use only constants, and compiler can define error.

public void DoSomething(Shape s) when s is Circle && s.Radius>10{}
public void DoSomething(Shape s) when s is Circle && s.Radius>12{}

I think, compiler can find it ambiguity.

[HaloFour]

@bondsbw

I'm still hung up on the runtime overload resolution. IMO that kills this proposal outright.

Indeed, other proposals to try to bring pattern matching into the method signature have been declined specifically for that reason. This proposal seems to gloss over a lot of the specific details and only really mentions either case guards or recursive patterns. But if it approaches trying to match on types themselves via overloads that would effectively result in changing behavior.

@Malanin

Pattern matching already only works on constants and lexical ordering is already a critical concern. A pattern guard (or recursive pattern) could be any combination of conditions. It'd be impossible (and certainly undesirable) to have the compiler assume in what order they should be matched.

[DavidArno]

@Malanin

But I don't understand why do you prefer write more code ... Just compare this two examples. My proposal make code cleaner, shorter and more readable.

Your first example clearly separates out one method with responsible for selecting the required functionality. from a set of methods, each of which performs a specific function. This is highly cohesive; each method has its own responsibility and that responsibility sits solely with that method. Sure, it's a bit longer; sometimes more code is better. I also feel that it's far cleaner, readable and most important, reasonable, ie it's easier to reason ("execute in your head") highly cohesive code.

[QsROg8320]

@HaloFour, do you think formal verification can't solve these problems?

[HaloFour]

@Malanin

do you think formal verification can't solve these problems?

No, I don't. The two following comparisons are perfectly legal and can match in either order. There is no "correct" way to order them.

public void foo(Shape s) when s is Rectangle && s.Width > 100 { }
public void foo(Shape s) when s is Rectangle && s.Area > 150 { }

Reordering those methods in the class should not change the business logic. And, as mentioned, with partial types it would be completely impossible for the compiler to assume an order at all.

[QsROg8320]

@HaloFour,

The two following comparisons are perfectly legal and can match in either order

Yes, and so that's why it wil be Compilation error. Compiler can use formal verification and find this error.

[HaloFour]

@Malanin

Yes, and so that's why it wil be Compilation error.

If that kind of match would result in a compiler error then I think that this feature would not only be useless but also exceptionally frustration as in my opinion the vast majority of pattern matching expressions will result in this case.

Would the compiler also fail in the case of non-exhaustive matches?

[gafter]

This is a dup of #1468

In my opinion, this is something we would likely never do in C#. The separation of declaration (signature) from implementation is something we value highly.

[alrz]

Would be great if we only could use void-returning methods in switch expressions..

void M(object o) => o switch
{
  T1 t => M(t),
  T2 t => M(t),
};

Nobody has expected the same in ternaries but with switch expressions the limitation is more visible.

[bbarry]

While I dislike the idea of some sort of property condition, being able to separate out method declarations for instances of a closed algebraic datatype is somewhat appealing:

Given ADT pseudo-syntax:

public abstract sealed class Option<T>
{
    public class None();
    public class Some(T value);
}

It could be useful to encode the switch directly into a method signature and make it a compiler error when you miss an option:

public class MyThing
{
    void M<T, Option<T>>(None x) => Console.WriteLine("None"); // compiles as void M<T>(Option<T>.None x)
    void M<T, Option<T>>(Some x) => Console.WriteLine("value: {0}", x); // void M<T>(Option<T>.Some x)

    //generated:
    void M<T>(Option<T> x) 
    {
        switch (x)
        {
        case Option<T>.None x1: M(x1);
        case Option<T>.Some x2: M(x2);
        }
    }
}

Really though this would be little more than a syntax suggestion for #486 at that point.

[CyrusNajmabadi]

Your first example is look good for two cases, but if we have 10 cases your code will be unreadable.

That code looks perfectly readable to me.

[CyrusNajmabadi]

Having a small public method that does the switch, and a bunch of private methods that handle each case is exactly what I'd want.

Or heck, if you don't want the clutter in the class, just use local functions. :)

[CyrusNajmabadi]

Yes, and so that's why it wil be Compilation error. Compiler can use formal verification and find this error.

I don't like this approach because i have no idea what it means. For example, say i have this:

class Base
{
    public virtual void DoSomething(Shape s) when s.Area > 10 { ... }
}

class Derived
{
    public override void DoSomething(Circle c) when c.Radius > 20 { ... }
}

What on earth does this mean? Types have invariance/covariance/contravariance. I can understand how things would work and which of those sorts of changes would be allowed. I have no idea what it means for this pattern restriction to be part of the signature of this method.

I also have no idea how this would work for metadata. So you have two DoSomething(Circle c) with different pattern restrictions. How would that possibly work in IL? How would another language understand this and call the right methods?

[QsROg8320]

@HaloFour Ok, I was wrong. I'm trying to solve this problem. The only thing I can think of is priority, but I don't like it.

[Priority(1)]
public void DoSomething(Shape s) when s is Circle && s.Area > 10 { ... }
[Priority(2)]
public void DoSomething(Shape s) when s is Circle && s.Radius > 20 { ... }

Or something like that:

public void DoSomething(Shape s) when 1:  s is Circle && s.Area > 10 { ... }

public void DoSomething(Shape s) when 2: s is Circle && s.Radius > 20 { ... }

[AustinBryan]

Honestly, this would mean that you can't use the same class more than once which to I think really defeats the point. Also, I'm sure having prorities for overloading has been denied before and I bet if they are added, they'll become a code smell right away. C# hasn't needed it up until now and I don't think it should