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: Arrow syntax (expression bodied return) for switch statements #544

Closed
GerjanOnline opened this issue May 5, 2017 · 51 comments
Closed

Comments

@GerjanOnline
Copy link

GerjanOnline commented May 5, 2017

What I'm writing now:

   	public String GetActionStateDescription(ActionState actionState)
   	{
   		switch (actionState)
   		{
   			case ActionState.Open:
   				return "In wachtrij";
   			case ActionState.Processing:
   				return "In verwerking";
   			case ActionState.Failure:
   				return "Mislukt";
   			case ActionState.Finished:
   				return "Voltooid";
   			default:
   				return "Onbekend";
   		}
   	}

What I would like to write:

	public String GetActionStateDescription(ActionState actionState)
	{
		switch (actionState)
		{
			case ActionState.Open => "In wachtrij";
			case ActionState.Processing => "In verwerking";
			case ActionState.Failure => "Mislukt";
			case ActionState.Finished => "Voltooid";
			default => "Onbekend";
		}
	}

I know about the C# 7.0 pattern matching features and the possible upcoming advanced pattern matching features in 7.1/8.0 but I hope this will be possible too.
There were some ideas to come up with a new match keyword that is expression based (see dotnet/roslyn#5154), maybe this case fits into this design. However I'm not quite confident the match keyword wil make it into C#. If that's the case, please consider this as an addition to the switch statement.

@GerjanOnline GerjanOnline changed the title Arrow syntax (expression bodied return) for switch statements Proposal: Arrow syntax (expression bodied return) for switch statements May 5, 2017
@DavidArno
Copy link

DavidArno commented May 5, 2017

You are correct, this is what the match expression is indended for, and might look something like:

public String GetActionStateDescription(ActionState actionState) =>
    actionState match (
        case ActionState.Open: "In wachtrij",
        case ActionState.Processing: "In verwerking",
        case ActionState.Failure: "Mislukt",
        case ActionState.Finished: "Voltooid",
        case *: "Onbekend"
    );

Whether the keyword is match or switch though makes little difference, so your suggested syntax is as good as any.

Currently it is unclear if this feature, and the rest of the missing pattern matching features, will ever be added to the language though, as the milestone for Champion "Pattern Matching" has slipped from 7.1, to 7.2 and is currently labelled as 7.X.

@lachbaer
Copy link
Contributor

lachbaer commented May 5, 2017

But then there shouldn't be two syntaxes for switch and match for the return case. When a return keyword is omitted it should be written in arrow syntax.

public String GetActionStateDescription(ActionState actionState) =>
    actionState match (
        case ActionState.Open => "In wachtrij",
        case ActionState.Processing => "In verwerking",
        case ActionState.Failure => "Mislukt",
        case ActionState.Finished => "Voltooid",
        case * => "Onbekend");

@casperOne
Copy link

I like the essence of the proposal, but I don't like specializing on the return value.

I feel @DavidArno example using match is more intuitive as well as expansive for other cases than return.

There are many times where I want to assign to a variable instead of return, but I'd still like to take advantage of any shorthand the language offers.

@DavidArno
Copy link

@casperOne,

Well spotted. I'd missed the fact that the OP's example is missing return before switch (actionState). That return (or an assignment to a variable in your example) would be needed.

@lachbaer
Copy link
Contributor

lachbaer commented May 5, 2017

My comment above shall be independent on the return keyword, just any case where switch or return return a value from their context, albeit an actual return or assignment (or part of any other expression). Expressed differently: when something gives back a result, then the arrow syntax should be used or the return keyword. That would make for a smooth experience.

That said, yield => as synonym for yield return should be adopted as well, on behalf of this proposal's idea.

@CyrusNajmabadi
Copy link
Member

Currently it is unclear if this feature, and the rest of the missing pattern matching features, will ever be added to the language though, as the milestone for Champion "Pattern Matching" has slipped from 7.1, to 7.2 and is currently labelled as 7.X.

We feel like these features are too large for a point release. But we're still very interested in them. Of course, nothing about the future can ever be confirmed since all sorts of things may change our plans. But pattern matching is still an area of deep interest for the team and the LDM.

@CyrusNajmabadi
Copy link
Member

Oh, and i should mention: of personal interest for me. I absolutely want nested patterns and proper expression-based match constructs. But it will take a little time to get there. We laid good foundation for this. And i think we have strong ideas on how we can accomplish the rest. Now we just have to appropriately schedule given the umpteen kajillion other things we also want to do.

@lachbaer
Copy link
Contributor

lachbaer commented May 8, 2017

I have just wanted to write

if (left == null || right == null) return false;

and mindlessly typed: 😄

if (left == null || right == null) => false;

That's not too bad, is it? It absolutely fits in the other arrow expression cases and to this proposal here. This can be especially good for those who normally put every single statement in a block. If however arrow-syntax is allowed by company style guides, whether the arrow is put on a second line or not, it reduces the occurences of braces and shortens the length of the code.

A single return on a condition is very common, e.g. upon condition checking at the beginning of a method. Imagine how nicely they can look with this 😊

(I'm not sure whether to make a new proposal for this or to keep it here. Guess here is an appropriate place.)

@lachbaer
Copy link
Contributor

lachbaer commented May 8, 2017

Another sample to picture it:

    public static bool operator ==(ValueHolder left, ValueHolder right)
    {
        if (!(left is null || right is null)) => left.Value == right.Value;  
        else => false;
    }

@CyrusNajmabadi
Copy link
Member

why not just do:

=> left?.Value == right?.Value

?

@DavidArno
Copy link

DavidArno commented May 8, 2017

Another sample to picture it

As an aside, @lachbaer, your code will return false for null == null.

@CyrusNajmabadi's code is both simpler and works better. Just saying... 😀

@DavidArno
Copy link

@CyrusNajmabadi,

We feel like these features are too large for a point release.

This is disappointing. Surely the whole idea behind point releases is to introduce features in an incremental way. We have the foundations for pattern matching in place, then 7.1 could (should) introduce some small addition this this etc, building up to match and active patterns in 7.X/8.0.

Aside from poor folk confused over how to get C# 7 to compile (due to tuples being in a nuget package and extra stuff needing to be done to make MVC work with it), the vast majority of questions on Stack Overflow regarding C# 7 have been about pattern matching. It's already a very popular feature and will genuinely benefit the majority of developers. It seems a no-brainer to me that completing that feature should be the team's priority.

@lachbaer
Copy link
Contributor

lachbaer commented May 8, 2017

@DavidArno

As an aside, @lachbaer, your code will return false for null == null

Yes, if one side is null then the whole comparision is false. It shall only be true if both operands have a value. That's correct the way it is 😉

@CyrusNajmabadi Your code would give true for null == null, see above 😁

@CyrusNajmabadi
Copy link
Member

Surely the whole idea behind point releases is to introduce features in an incremental way.

No. The idea behind point releases is to deliver improvements that we can reasonably provide. As I've mentioned numerous times to you, if we don't think we have the time to do something, we'll push it out. When we looked at patterns we felt it was too large to do given the time we have for point releases.

It seems a no-brainer to me that completing that feature should be the team's priority.

You're conflating concepts. Something can be a priority, but not be a point feature. Things can be point features if we realistically feel we can deliver it in a point release. That is not the case here, full stop.

Language features take a lot of time and effort. We want to fully design things. We need to implement it. We need to use it and revise things based on what we've learned. We have an entire IDEs worth of features that need to be updated accordingly. The stuff that can fit in point releases are the features that can do all of that in the incredibly small amount of time available. Patterns are not that. I'm not sure how i can make that any clearer :)

@CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi Your code would give true for null == null

Yes. That seems appropriate. Given that's the semantics of the language today for nulls.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented May 8, 2017

Yes, if one side is null then the whole comparision is false.

So null == null would be false, but null != null would also be false? So !(null == null) would not be equivalent to null != null?

It seems like you've created an extraordinarily weird type, and you want the language to provide constructs to help you with that. That's not generally how we do language features. We want to make it easy to create well behaved types.

@lachbaer
Copy link
Contributor

lachbaer commented May 8, 2017

@CyrusNajmabadi This is the topic about => isn't it??? 😯

@lachbaer
Copy link
Contributor

lachbaer commented May 8, 2017

@CyrusNajmabadi

So "null == null" would be false, but "null != null" would also be false? So "!(null == null)" would not be equivalent to "null != null"?

if (left is null || right is null) return left.Value == right.Value; can never be null != null or !(null == null). This is no weired type or so. Now I guess you're lost a bit!

@DavidArno
Copy link

@CyrusNajmabadi,

The stuff that can fit in point releases are the features that can do all of that in the incredibly small amount of time available. Patterns are not that. I'm not sure how i can make that any clearer :)

You are OK, it's crystal clear: you don't want to add more features to pattern matching on an incremental basis. I get that; I just think that decision is disappointing. But I'll get over it 😀

@jnm2
Copy link
Contributor

jnm2 commented May 8, 2017

@lachbaer C# null semantics are nothing like SQL null semantics and never have been.

@jnm2
Copy link
Contributor

jnm2 commented May 8, 2017

Well, except for this singular case:

There is also the odd result that (int?)null == (int?)null evaluates to true but (int?)null <= (int?)null does not.

(http://stackoverflow.com/a/23787253)

@CyrusNajmabadi
Copy link
Member

You are OK, it's crystal clear: you don't want to add more features to pattern matching on an incremental basis. I get that;

We only want to add features incrementally that we think we have the time to do properly. If we felt that was the case for patterns, we would do so.

For example, we are adding the capability for patterns to work better with type parameters (as you've seen in bugs you've filed yourself). That was small enough to fit. Other stuff in patterns is much larger, and we did not feel we would be able to sufficiently get the quality high enough for a point release.

@Thaina
Copy link

Thaina commented May 9, 2017

I would prefer switch expression and return switch with your proposal about arrow in case

	public String GetActionStateDescription(ActionState actionState)
	{
		return switch(actionState)
		{
			case ActionState.Open => "In wachtrij";
			case ActionState.Processing => "In verwerking";
			case ActionState.Failure => "Mislukt";
			case ActionState.Finished => "Voltooid";
			default => "Onbekend";
		}
	}

I also prefer this syntax than match because it not add new keyword

@lachbaer
Copy link
Contributor

lachbaer commented May 9, 2017

I also prefer this syntax than match because it not add new keyword

As far as I understood match does the case differently than switch, it's a new concept and therefore a new keyword.

I once proposed a conditional return in #453 (closed) and now arrow expression syntax for if in #559.

I think for all those expressions it is important that they have return keyword preceding and at the end of the statement there has been a definite return, no code behind it is ever reached. Maybe we can move over some of the discussion from #559 to here, because if-else and switch are so similar.

public static bool operator ==(Fraction left, Fraction right)
{
    return if (!left.IsValid ^ !right.IsValid)
        => false;
    else 
        => left.value == right.value;
}

Before the discussion goes, why not using ?: in this case: some people like it more verbose in some places. If switch were supporting this return switch statement, then if should also. That can be discussed also here.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented May 9, 2017

If switch were supporting this return switch statement, then if should also.

Conditionals already have a statement form, and a short expression form. We don't need a third form for them :)

Switches, however, only have a statement form. There is a need (with ample precedent) for a simpler and shorter expression-form. The exact syntax is TBD on it. but it will likely look similar to several of the outlined options people have laid out above.

@lachbaer
Copy link
Contributor

lachbaer commented May 9, 2017

There is a need (with ample precedent) for a simpler and shorter expression-form.

But shorter does not mean to put an additional return in front and replacing the : of the cases by two characters =>. Then it must be something completeley different, like ? :.

@CyrusNajmabadi But read again my comment literally!

If switch were supporting this return switch statement, then if should also. That can be discussed also here.

"If" switch "were" layout like proposed here, then if should also. Everything else is a mess and not consistent at all! Whether there already exists another short form or not. How will you logically explain to every newbie that you can write return switch ... but not return if ..., without any other further sideviews? Those two are just too similar when a users sees a case as a shortform of if ... if else ... else, as it is mainly explained in every single book I have.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented May 9, 2017

Then it must be something completeley different, like ? :.

I don't see why that's the case at all. Right now pattern matching in an expression context is inconvenient due to 'switch' being a statement. We just need a suitable expression form. It doesn't have to be "something completely different"

"If" switch "were" layout like proposed here, then if should also

I disagree. 'if' already has an expression form. We don't need to introduce a new one.

How will you logically explain to every newbie that you can write return switch ... but not return if ...

The same way we explain everything in the language today. :)

@lachbaer
Copy link
Contributor

lachbaer commented May 9, 2017

My immediate idea: ?* : : :

public String GetActionStateDescription(ActionState actionState)
    => actionState ?*
          ActionState.Open => "In wachtrij"
        : ActionState.Processing => "In verwerking"
        : ActionState.Failure => "Mislukt"
        : ActionState.Finished => "Voltooid"
        : "Onbekend";

@CyrusNajmabadi
Copy link
Member

Those two are just too similar

switches and 'if-statements' share almost nothing in common. I see no reason why people would expect the patterns available in one to be available in the other. I also don't know what books you're referring to. I was never taught to think of switches and ifs as being syntactically similar to each other. :)

I don't think i've ever thought to myself that "if an 'if' had something, then a 'switch' should have the same" (or vice-versa).

@CyrusNajmabadi
Copy link
Member

actionState ?* ActionState.Open

This would require a lot of lookahead. The above is already legal syntax.

I don't think we'd introduce an operator for this. We'd likely either use the term 'match' or we'd allow for a 'switch' to be used in an expression context. i.e.:

return switch (expr) {
     case X: value;
     case Y x: value;
};

@lachbaer
Copy link
Contributor

lachbaer commented May 9, 2017

@CyrusNajmabadi No offense, but it would really be nice to have some other opinions on this from the team. As I have told you so many times already, I (me personally) think that you have a way too much technical perspective on those things. You lack the perspective of the standard user. Really, no offense! 😃 But that's just who you are.

@CyrusNajmabadi
Copy link
Member

@lachbaer That's how we design the language.

@lachbaer
Copy link
Contributor

lachbaer commented May 9, 2017

return switch (expr) {
     case X: value;
     case Y x: value;
};

...lacks a default case. The switch might fall through and not return.

@CyrusNajmabadi
Copy link
Member

Yup. I would certainly want teh user to be able to write that. If, at runtime, the switch fell through, you'd get a runtime error.

@lachbaer
Copy link
Contributor

lachbaer commented May 9, 2017

I would like the opinion of others on this!

All alternatives beside:

Would it be consistent to the language if return switch was allowed that return if ... else was allowed also?

Think also back to your earlier days and try to recall how you learned programming and what books told you.

@CyrusNajmabadi
Copy link
Member

@lachbaer The relevant quesiton is not "would it be consistent", it's:

"Is there enough value here to warrant creating a expression-form for 'if statements' when ?: already exists?"

Language proposals can be 100% consistent, and yet still not be something we would take. See my previous comment in the day about initial-negative-value for features and why we need features to deliver enough to warrant doing them. Simply making things more consistent is not enough.

@Richiban
Copy link

Richiban commented May 9, 2017

@lachbaer If we were designing a new, C#-like language from the ground up then, yeah, I'd make everything an expression--including if. Unfortunately we live in this situation where the C# team are having to retrofit this stuff onto an existing language in a way that feels right, without ever (deliberately) breaking existing code.

This means that, no matter how much sense it might make to some of us that everything is an expression, the existence of the ternary expression z = b ? x : y precludes there ever being an z = if (b) x else y syntax.

I believe the team are doing the right thing here, given that one of their explicit goals is (and always has been) backwards compatibility. Barring some breaking-change language syntax overhaul where we could finally retire the legacy constructs I don't think there's any reason to introduce a new concept that is literally just a re-implementation of something we already have.

@Thaina
Copy link

Thaina commented May 9, 2017

Well, more and more I think about this, more and more I remnded about #249

@lachbaer
Copy link
Contributor

lachbaer commented May 9, 2017

@CyrusNajmabadi @Richiban Well expained, now.

To recap:

  • no additions, even if they would make sense in terms of consistency, just for the sake of consistency
  • mixing styles (short ?: vs. full = switch...) is allowed when the new construct is better readable then

With the given explanations I can live with that. 😊

@lachbaer
Copy link
Contributor

lachbaer commented May 9, 2017

To get back to the initial issue. I don't mind whether a switch expression uses : or => after the case. But that should be consistent with a probable match expression and both should use the same syntax.

Because match and the switch statement use the colon everywhere, I'll vote for that.

Also I upvote for a switch expression on the whole that looks exactly like the current switch statement. Of course a default case must be defined in the expression. That case should be the last, but need not to be the last (exactly as if it were a standard switch statement with default case).

@Richiban
Copy link

Richiban commented May 9, 2017

@lachbaer

Of course a default case must be defined in the expression

Just a minor nitpick here: I'd assume that any 'upgrades' to the switch statement would include completeness checking, which means that the default case can be omitted if the compiler can prove that one of the branches will execute at runtime. Here's the simplest example (which is kinda nuts):

bool b = ...;

switch(b)
{
    case true: Console.WriteLine("True"); return;
    case false: Console.WriteLine("False"); return;
    default: Console.WriteLine("WTH?"); return; // Currently C# makes us write this default
}

With completeness checking the compiler is satisfied that, at runtime, either the true or false branch is guaranteed to run and therefore no default is needed.

@jnm2
Copy link
Contributor

jnm2 commented May 9, 2017

@Richiban

bool x = LolBool.Value;

switch (x)
{
    case true:
    case false:
        break;
    default:
        throw new Exception("lol");
}

public static class LolBool
{
    public static readonly bool Value = new LolBoolUnion(0b10000000).BoolValue;

    [StructLayout(LayoutKind.Explicit)]
    private struct LolBoolUnion
    {
        [FieldOffset(0)]
        private readonly byte byteValue;
        [FieldOffset(0)]
        public readonly bool BoolValue;

        public LolBoolUnion(byte byteValue) : this()
        {
            this.byteValue = byteValue;
        }
    }
}

@Richiban
Copy link

Richiban commented May 9, 2017

@jnm2 I don't even...

I don't even know what's going on here. Have you somehow got a bool and a byte in the same bit of memory?

@lachbaer
Copy link
Contributor

lachbaer commented May 9, 2017

@jnm2 up to now I didn't know how to realize unions in C# 😄 (well, never needed it 😉)

@Thaina
Copy link

Thaina commented May 9, 2017

@Richiban Yes you could do that. You can even modify readonly field if you set FieldOffset on another public field in the same offset

We can even layout 8 byte into fieldoffset 0 - 7 and long on fieldoffset 0. That let us modified each byte of the long

@jnm2
Copy link
Contributor

jnm2 commented May 9, 2017

@Richiban You got it. Bools are jitted to bytes for efficiency, and I overlapped one with a byte field.

@Richiban
Copy link

Richiban commented May 9, 2017

@jnm2 Wow, so that's how it appears to be neither true nor false?

What happens if you use that in an if statement?

@tannergooding
Copy link
Member

@Richiban, the below produces "x", "x == true", "other".

bool x = LolBool.Value;

if (x)
{
	Console.WriteLine("x");
}
else
{
	Console.WriteLine("!x");
}

if (x == true)
{
	Console.WriteLine("x == true");
}
else
{
	Console.WriteLine("x != true");
}

switch (x)
{
	case true:
	{
		Console.WriteLine("true");
		break;
	}

	case false:
	{
		Console.WriteLine("false");
		break;
	}

	default:
	{
		Console.WriteLine("other");
		break;
	}
}

@jnm2
Copy link
Contributor

jnm2 commented May 9, 2017

bool x = LolBool.Value;

bool trueVariable = true;
if (x == trueVariable)
    Console.WriteLine("This does not happen because 0b10000000 != 0b00000001");

const bool trueConst = true;
if (x == trueConst)
    Console.WriteLine("But this does, because it's reduced to `if (x)` which is ultimately a non-zero check");

@jnm2
Copy link
Contributor

jnm2 commented May 9, 2017

Now to be perfectly honest with you, I'm fine if default is not required for bools and any bool that is not true or false causes a runtime error.

@gafter
Copy link
Member

gafter commented Apr 19, 2018

The extension of further pattern-matching constructs for C# 8, currently prototyped in the Roslyn branch features/recursive-patterns, supports these use cases. With the currently implemented syntax (open to revision), you'd write

public String GetActionStateDescription(ActionState actionState) =>
    actionState switch {
        ActionState.Open => "In wachtrij",
        ActionState.Processing => "In verwerking",
        ActionState.Failure => "Mislukt",
        ActionState.Finished => "Voltooid",
        _ => "Onbekend"
    );

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

10 participants