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

Adding statement seperator to if-else, for, etc. clauses #559

Closed
lachbaer opened this issue May 8, 2017 · 66 comments
Closed

Adding statement seperator to if-else, for, etc. clauses #559

lachbaer opened this issue May 8, 2017 · 66 comments

Comments

@lachbaer
Copy link
Contributor

lachbaer commented May 8, 2017

Update

This is no longer about Arrow expression syntax to if-else clauses.

The intent was to eliminate the reasons for guidelines for embracing every statement that follows an if, while or for (etc.) statement.

Please see #559 (comment) for a better catchup explanation.

[Old initiating post:]

Arrow expressions are awesome!

They let code look so good, short and intuitive. I'd like to have them for if-clauses as well.

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

Why not use return? Because many style guides prescribe the use of blocks after every if, what would result in the much longer:

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

But the same style guides allow the use of arrow syntax, what would result in the first 5-liner.

Especially when some conditions must be met at the beginning of a method and early returns are common then this syntax could shorten code and enhance readability.

Questions

  • Should it be allowed in void contexts as well? void llambda expressions exists.
@jnm2
Copy link
Contributor

jnm2 commented May 8, 2017

If there is more than one line, I think readability disappears without seeing the word return.

@orthoxerox
Copy link

public static bool operator ==(Fraction left, Fraction right) 
    => (!left.IsValid ^ !right.IsValid) 
        ? false 
        : left.value == right.value;

@lachbaer
Copy link
Contributor Author

lachbaer commented May 8, 2017

@orthoxerox What if the method had no else and the method block continues? Please don't take the example literally, it's just there to visualize the concept.

@CyrusNajmabadi
Copy link
Member

This is an interesting idea. But the problem, to me, is that => expr doesn't necessarily mean { return expr; }. It can also just mean { expr; }. For example, one might have:

if (x) => Foo()
else => Bar();

Why should this return if the user just wanted the different invocations to just execute.

And if this really is only to make returning better, then why do we need this, when we also have the simpler:

return x ? Foo() : Bar();

?

@lachbaer
Copy link
Contributor Author

lachbaer commented May 8, 2017

@CyrusNajmabadi

And if this really is only to make returning better, then why do we need this, when we also have the simpler

See #559 (comment).

It can also just mean { expr; }

That's what I meant by the question "Should it be allowed in void contexts as well?".

It shall be a shorthand for return'ing and thereby reducing the number of additional braced blocks that only have a single return statement in it. Many programmers or companies prefer to put if-else-while-for bodies in full blocks (me, for production code, too) while at same time allowing arrow syntax. That's the whole point behind this idea.

Even when put in seperate lines, it reduces 4 lines of { / } with only that one character in it.

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

What's the point behind arrow expression syntax if not reducing lines or having a different (functional) "look"?

@lachbaer
Copy link
Contributor Author

lachbaer commented May 8, 2017

Oh, and besides, maybe the programmer wants the explicit if and else statements for his fellow workers that do design or so and only have rudimentary programming knowledge, i.e. have never seen ? : syntax. Nonetheless the arrow-syntax is quite intuitive!

@HaloFour
Copy link
Contributor

HaloFour commented May 8, 2017

This proposal is incredibly unintuitive. An arrow expression syntax supported by if statements would appear to convert only that statement into an expression. Having it return from the entire method makes absolutely no sense, not in C-style languages or in functional languages.

If you want an expression, use the existing syntax. If you want statements, use the existing syntax. In neither case is the existing syntax onerously verbose. And when it comes to style rules, believe me that the same places that would demand blocks for return statements would absolutely forbid this.

@CyrusNajmabadi
Copy link
Member

@lachbaer For methods/lambdas/locals, there is a clear understanding of what "=>" should mean. That's because the => attaches directly to the thing which is either void or non-void.

With 'if' statements you're now making things confusing because you can totally have an if-statement in a void or non-void member. But it would then be the member which affected how => was treated inside the if.

That is not at all what i would expect.

@CyrusNajmabadi
Copy link
Member

i.e. have never seen ? : syntax.

I'm confused. They won't have seen this syntax (despite it being in C# since 1.0), but they will have seen => (which we only added for lambdas in 3.0 and for other members much later) ?

We already have syntax which is more concise and more uniform for this use case today. I would much rather people just use that :)

@lachbaer
Copy link
Contributor Author

lachbaer commented May 8, 2017 via email

@lachbaer
Copy link
Contributor Author

lachbaer commented May 8, 2017 via email

@CyrusNajmabadi
Copy link
Member

Too long and nested blocks are counter productive.

so use ?:

@lachbaer
Copy link
Contributor Author

lachbaer commented May 8, 2017 via email

@jnm2
Copy link
Contributor

jnm2 commented May 8, 2017

@lachbaer

Mr. Other Team Member That Is No Programmer But Has Access To My Code For What Ever Reason

is going to be tripping on way more important, fundamental issues than this.

@CyrusNajmabadi
Copy link
Member

What the hell is ? :...?

Use this as a teaching opportunity. If devs on your team don't know about constructs in C# 1.0 then it's a good time to teach them.

@svick
Copy link
Contributor

svick commented May 9, 2017

Wouldn't it be great to keep typing down your programmatic idea the shortest way possible to your thoughts, not the shortest written?

If I was writing the code from your example, my thoughts (admittedly influenced by knowing C#) would go something like:

If the validity of left and right are not the same, return false. Otherwise, compare their values for equality and return that.

That translates fairly nicely to:

if (left.IsValid != right.IsValid)
    return false;
else
    return left.value == right.value;

(left.value == right.value.return is even closer for the last line. That is not valid C#, but ReSharper does convert it to that.)

I don't see how does your proposal improve on that. (And if your coding style dictates braces, then presumably your company thinks some verbosity is worth it.)

Also, I do no understand how did you arrive at the "not xor not" expression, especially considering that the two negations cancel each other out.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

And if your coding style dictates braces, then presumably your company thinks some verbosity is worth it.

The reason that even simple return statements are wrapped in braces is that the return keyword consists of letters. If not put in an own block it could easily be overseen whether it belongs logically to the previous statement or whether it is independent. That's mainly the whole purpose of that coding style rule.

See the following example (no code formatting by purpose):

if (left.IsValid != right.IsValid)
return false;

if (left.IsValid != right.IsValid);
return false;

With a symbolic representation however it is clear even without braces that it belongs to the previous statement, no matter where it stands.

if (left.IsValid != right.IsValid)
:=> false;

if (left.IsValid != right.IsValid);
:=> false;

The last one would give a compile time error.

So in the case of a symbolic return statement, e.g. :=> that cannot be used on its own, there is no more need for blocks, it is clearer to spot and reduces mistakes like the one above.

@CyrusNajmabadi
Copy link
Member

if (left.IsValid != right.IsValid);
return false;

C# intentionally warns you if you do that:

Warning CS0642 Possible mistaken empty statement

@CyrusNajmabadi
Copy link
Member

If the validity of left and right are not the same, return false. Otherwise, compare their values for equality and return that.
That translates fairly nicely to:

if (left.IsValid != right.IsValid)
    return false;
else
    return left.value == right.value;

And i would simplify that further to:

return left.IsValid == right.IsValid &&
       left.value == right.value;

Which, conveniently, is what our 'Generate-Equals' code helper will even generate for you here :D

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

@CyrusNajmabadi

C# intentionally warns you if you do that: Warning CS0642 Possible mistaken empty statement

Yes, and that's why we start using braces again... 🙄 But that was not the point. I wanted to show why braces are better used in this case, something I try to avoid by this topic.

Just another idea, that considers everything written above.

When allowing a tiny, little : after the if-condition additionally to the current syntax, then the blocks can be replaced and everything loosens up.

if (left.IsValid != right.IsValid)
    : return false;

No braces needed any more.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

Which, conveniently, is what our 'Generate-Equals' code helper will even generate for you here

Sorry that I am soooo dumb! 😢 I have never heard of that one. Neither in books, nor before now. But probably every other one on the world knows, but me...

@CyrusNajmabadi
Copy link
Member

It's coming in 15.3:

image

image

image

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

And i would simplify that further to

That's what you came up with after thinking a bit. But do you instantaneously think that way when programming? If yes, good for you, if no, welcome to the ordinary folks.

We cannot review every single line of the code to find a way that suits better. We need to keep the costs per line down. Especially when not being on a team and programming mainly alone. I have to focus on my algorithms, my class layout, etc. and I simply want the language to support me by not having to think about every single optimization.

I'm putting all statements - yes, return too - in blocks after if, so that I will not oversee them. But that enlongates the code and it makes me scroll much, obfuscating my view on the total.

Before I write anything further, @CyrusNajmabadi do you understand my point?

@CyrusNajmabadi
Copy link
Member

and I simply want the language to support me by not having to think about every single optimization.

In my opinion, the less in the language, the more your goal is supported. Many of your suggestions have been about introducing many new ways to do things that are already possible and now idiomatic in the language now that it's been around 15+ years.

When you introduce new ways to do things, it adds a mental tax to everyone as now there is a need for people to know and internalize all the different ways to do things.

Conversely, if there are just a few ways to accomplish what you want, then it becomes clearer and easier to know what to write, and it becomes easier to review the code around you.

But that enlongates the code and it makes me scroll much, obfuscating my view on the total.

You're avoiding constrcts the language has, and the ecosystem and community use. But then you don't like the impact it has on your code. And then you propose new constructs to help address that :)

Talking to you, it often feels as if you're fighting against what the language is, and you're really trying to get us to make the language into what you want it to be. But that's not really what we're going to do. IMO, if you're using a language, it's worthwhile to learn and embrace its idioms, and find good ways to use them to help support your programming goals, rather than work against them.

do you understand my point?

I think so.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

if (left.IsValid != right.IsValid)
    : return false;

I must say, that's my best idea up to now! Maybe I'm prototyping that and simply eat that colon token when present. For while, else, foreach, for also. That is a neat, short syntax to prevent excessive bracing! And this could really be of practical use for many, I think!

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

Talking to you, it often feels as if you're fighting against what the language is, and you're really trying to get us to make the language into what you want it to be

I have ideas that come from my everyday use. Those ideas may be really odd sometimes, but they often have a deeper lying reason. Discussing them may help to develop better ideas or to discard them for the better.

The deeper lying reason for this topic is that I find that heavy bracing cumbersome (see above). That's most probably not my problem alone. But I don't want to omit them for the same reasons that many coding styles demand it.

Another topic made me come up with this idea of arrow expressions for if-else that would avoid the braces by looking a bit like a (non-braced) llambda expression or alike. I see that that syntax is not adequate, also for some semantic reasons. And discussing that made me come up with the idea from the previous post.

@CyrusNajmabadi will probably say "you can already write if (cond) return without a colon, there is no need for a new syntax. But then he hasn't understood the deeper reason for that proposal, that I have broadly explained now.

@bondsbw
Copy link

bondsbw commented May 9, 2017

Is that return false supposed to be in an else clause?That's what it looks like, considering the ternary operator.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

hat's what it looks like, considering the ternary operator.

Good point. But that may be just a matter of accustoming.

// This is just a visual example
if (left.IsValid != right.IsValid): return false;

Now it doesn't directly look like the else clause of a ternary. Also in other representations a : colon marks the continuation of the previous line. It is based on the : of a case xxx: or default:.

Or do you have another idea or token that achieves the purpose?

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

if (left.IsValid != right.IsValid)
    ^ return false;

Looks like a error message on the command line.

if (left.IsValid != right.IsValid)
    -> return false;

Pointer? Nah, looks ugly.

if (left.IsValid != right.IsValid)
    [ return false; ]

Same as with braces, and may conflict with "attributes everywhere".

if (left.IsValid != right.IsValid)
    _ return false;

Too small, not noticable enough

if (left.IsValid != right.IsValid)
    \ return false;

Too much similarity to //

if (left.IsValid != right.IsValid)
    :: return false;

Well, this could be an alternative.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented May 9, 2017

"I want a syntax with a different optical apperance, like:"

if (left.IsValid != right.IsValid)
    <- false;

And then another person comes along and says, " I simply want the language to support me ", and i don't like any of those, give me:

=> false when left.IsValid != right.IsValid;

:)

And so on, and so forth. Is there a point at which you don't do these? Or do we keep adding this stuff because the existing options are not sufficient for some customers?

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented May 9, 2017

(Sarcastically) Or put differently: There already exists a possibility to view the code that I refer to. Why should I copy and paste it

Because i don't want to see your change applied to part of the method. I want to see what the entire method would look like applying your code change. And i'm giving you the opportunity to do this, instead of me guessing at your language feature, potentially getting it wrong, and frustrating you because i'm now discussing something different than what's in your mind.

If you like you can follow that link and put your own copy here in that thread.

Fine. Here's what i think it would look like based on how you've described your language feature so far:

  {
        Debug.Assert(left != null);
        Debug.Assert(right != null);

        if (left.HasAnyErrors || right.HasAnyErrors)
            :: return null;

        // SPEC VIOLATION: see method definition for details
        ConstantValue nullableEqualityResult = TryFoldingNullableEquality(kind, left, right);
        if (nullableEqualityResult != null)
            :: return nullableEqualityResult;

        var valueLeft = left.ConstantValue;
        var valueRight = right.ConstantValue;
        if (valueLeft == null || valueRight == null)
            :: return null;

        if (valueLeft.IsBad || valueRight.IsBad)
            :: return ConstantValue.Bad;

        if (kind.IsEnum() && !kind.IsLifted())
            :: return FoldEnumBinaryOperator(syntax, kind, left, right, diagnostics);

        // Divisions by zero on integral types and decimal always fail even in an unchecked context.
        if (IsDivisionByZero(kind, valueRight))
        {
            Error(diagnostics, ErrorCode.ERR_IntDivByZero, syntax);
            return ConstantValue.Bad;
        }

        object newValue = null;

        // Certain binary operations never fail; bool & bool, for example. If we are in one of those
        // cases, simply fold the operation and return.
        //
        // Although remainder and division always overflow at runtime with arguments int.MinValue/long.MinValue and -1 
        // (regardless of checked context) the constant folding behavior is different. 
        // Remainder never overflows at compile time while division does.
        newValue = FoldNeverOverflowBinaryOperators(kind, valueLeft, valueRight);
        if (newValue != null)
            :: return ConstantValue.Create(newValue, resultType);

        ConstantValue concatResult = FoldStringConcatenation(kind, valueLeft, valueRight, ref compoundStringLength);
        if (concatResult != null)
        {
            if (concatResult.IsBad)
                ::  Error(diagnostics, ErrorCode.ERR_ConstantStringTooLong, syntax);

            return concatResult;
        }

        // Certain binary operations always fail if they overflow even when in an unchecked context;
        // decimal + decimal, for example. If we are in one of those cases, make the attempt. If it
        // succeeds, return the result. If not, give a compile-time error regardless of context.
        try
            :: newValue = FoldDecimalBinaryOperators(kind, valueLeft, valueRight);
        catch (OverflowException)
        {
            Error(diagnostics, ErrorCode.ERR_DecConstError, syntax);
            return ConstantValue.Bad;
        }

        if (newValue != null)
            :: return ConstantValue.Create(newValue, resultType);

        if (CheckOverflowAtCompileTime)
            :: try
                :: newValue = FoldCheckedIntegralBinaryOperator(kind, valueLeft, valueRight);
            catch (OverflowException)
            {
                Error(diagnostics, ErrorCode.ERR_CheckedOverflow, syntax);
                return ConstantValue.Bad;
            }
        else
            :: newValue = FoldUncheckedIntegralBinaryOperator(kind, valueLeft, valueRight);

        if (newValue != null)
            :: return ConstantValue.Create(newValue, resultType);
        else 
            :: return null;
    }

If i got that wrong, then sorry. But that is why i asked if you could post it yourself :-/

Now, looking at that, it looks really wonky to me. It's a big mismatch of different styles and patterns for statements+embedded-statements.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017 via email

@CyrusNajmabadi
Copy link
Member

Yes, they are currently not sufficient!

So as long as someone complains and feels that the existing solutions are insufficient to them... we should just keep adding alternative ways to do these things? If we added your approach, and someone says it's "currently not sufficient"... then we just add what they want as well? And so on and so forth?

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017 via email

@CyrusNajmabadi
Copy link
Member

So I simply answer to that with a big ironic YES!

Alright, at least you're consistent. :)

I was hoping with this exercise that you would realize the problem inherent here. We simply are not going to take C# and keep adding redundant features just because someone wants it :)

After even 2-3 versions it would be an absolute mess. And I imagine you'd be one of many asking "why are there 6+ ways to just pick between two values and return them?"

As i mentioned before, any change to hte language has to be significantly valuable. And each time you introduce something redundant, you not only start out with your -1000 points, but you also have even more of a difficult time because a redundant feature weighs the language down even more by adding complexity, confusion, and bifurcation to the community.

--

As such, for future proposals, i would recommend workign to present the value of the proposal as more than just something you want because "[you] simply want the language to support [you] by not having to think about every single optimization."

That's not a sufficient reason for us to do something. And, as you've seen on many of your proposals, the community is not reacting to it well. Proposals should ideally be able to demonstrate their worth against the language as it exists today. Not against the subset of hte language you use because you've decided not to use certain parts of it.

If the feature has value whne compared to the entire language we have, i think you'll see a highly positive reaction from the community. However, if the feature just replicates what is already there, but which you don't like using, then i think you'll see little interest.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

@CyrusNajmabadi Do you know what "ironic" means? Meanwhile I have my doubts a bit.

here's what i think it would look like based on your language feature

I wouldn't do it for try (only blocks allowed!) neither for parts where the if is followed by more than one line. You could violate that, as you can violate other constructs already.

Anyhow, I think we must completely start over the discussion! Because you still haven't got the essence of it.

First: I don't want the syntax to be how I want it only!!!!!!! Never ever! Period!

If you still believe that then you are completely mistaken! You must break loose from that idea! I see a problem from a users perspective and I am making suggestions on how that could be resolved. The suggestion is and shall be a starter for the discussion.

The word "Proposal" does not occur in the topic.

This is a discussion about a concept. There is no concrete syntax that is proposed as being without any alternative.

You keep stating that there is already valid syntax. But if that is the last word from the team since 15 years, then why have you introduced expression bodies?

Why writing

int Age
    => CalculateAge(_dayOfBirth);

if you could already have that done by

int Age
{
    get
    {
        return CalculateAge(_dayOfBirth);
    }
}

Why introducing

int GetAge() =>
    CalculateAge(_dayOfBirth);

if there already was a valid syntax

int GetAge()
{
    return CalculateAge(_dayOfBirth);
}

It only saves two mostly empty lines, one keyword and adds two further characters.
And be honest, the main reason to introduce expression bodies, or why people use them, is to shorten their code.

Why do people want to shorten their code?

I made my point above already in several places (e.g. less scrolling). What's your answer?

Why is the current syntax not sufficient

I wrote that already many times, but, whew, again:

  1. Putting a single statement behind another statement is considered bad style (by me/team/company).
  2. Putting the same statement on the next line, with indention, is also considered bad style.
  3. Curly braced blocks shall be used in those cases.
  4. This leads to blocks with only one single statement (e.g. return) in it.
  5. This is a very, very, very common coding style around the world.

BUT, curly braces blow up the length of the code and produce unproductive and actual unneccessary space. They are only there to protrude the statement.

Alternative to current syntax

It would be great to have any way to protrude the statement without the surrounding braced block.

The initial idea was to add => for the case of return only. But that seems inappropriate for some reasons.

The second idea was to add : or :: that can precede that single statement. But the character seems to be to misleading. It could be any token that suits the community, | for example. ( @CyrusNajmabadi Besides if this syntax, that introduces a completely new construct, was implemented then there is no need for others to request just other tokens. Got that? )

New idea

The third (new) idea to achieve the goal of protruding the single statement without a braced block is to introduce a "blank token", that is composed of only one, two tops, characters. This token has no meaning and is simply eaten while parsing. It is a bit like the underscores in numbers (1_000_000).

To go with ASCII I'm using the backtick ` character , followed by a colon :. The backtick alone is too small, like a flyspeck. (Update: backtick alone looks fine, see #559 (comment))

if (left.IsValid != right.IsValid)
    ` return false;

(Also, the backtick could just make the following character be ignored. @CyrusNajmabadi Then everybody can use their own character, when that's what you want. 😀)

The highlight color should stay the standard color. It shall not be green like comments, so that the protruding character of that construct is preserved.

With this third idea no syntax change to the if and alike statement is done, but nevertheless the goal is achieved. The "blank token" (or whatever it should be called) is no trivia, it's a token, just without a semantic meaning.

Addendum: the backtick should be allowed in places where other statements and maybe start of expressions are allowed.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

However, if the feature just replicates what is already there, but which you don't like using, then i think you'll see little interest.

How shall I know that without consulting the community? And there are proposals from me that aren't closed yet and that aren't torn apart or have their likes.

But if it keeps being this way and brainstormes aren't welcome then it might be better for me to backtrack and shut up 😢

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

I must correct, the backtick character alone already serves the purpose enough.

https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs#L1415 with backtick token:

{
        Debug.Assert(left != null);
        Debug.Assert(right != null);

        if (left.HasAnyErrors || right.HasAnyErrors)
            ` return null;

        // SPEC VIOLATION: see method definition for details
        ConstantValue nullableEqualityResult = TryFoldingNullableEquality(kind, left, right);
        if (nullableEqualityResult != null)
            ` return nullableEqualityResult;

        var valueLeft = left.ConstantValue;
        var valueRight = right.ConstantValue;
        if (valueLeft == null || valueRight == null)
            ` return null;

        if (valueLeft.IsBad || valueRight.IsBad)
            ` return ConstantValue.Bad;

        if (kind.IsEnum() && !kind.IsLifted())
            ` return FoldEnumBinaryOperator(syntax, kind, left, right, diagnostics);

        // Divisions by zero on integral types and decimal always fail even in an unchecked context.
        if (IsDivisionByZero(kind, valueRight))
        {
            Error(diagnostics, ErrorCode.ERR_IntDivByZero, syntax);
            return ConstantValue.Bad;
        }

        object newValue = null;

        // Certain binary operations never fail; bool & bool, for example. If we are in one of those
        // cases, simply fold the operation and return.
        //
        // Although remainder and division always overflow at runtime with arguments int.MinValue/long.MinValue and -1 
        // (regardless of checked context) the constant folding behavior is different. 
        // Remainder never overflows at compile time while division does.
        newValue = FoldNeverOverflowBinaryOperators(kind, valueLeft, valueRight);
        if (newValue != null)
            ` return ConstantValue.Create(newValue, resultType);

        ConstantValue concatResult = FoldStringConcatenation(kind, valueLeft, valueRight, ref compoundStringLength);
        if (concatResult != null)
        {
            if (concatResult.IsBad)
                ` Error(diagnostics, ErrorCode.ERR_ConstantStringTooLong, syntax);

            return concatResult;
        }

        // Certain binary operations always fail if they overflow even when in an unchecked context;
        // decimal + decimal, for example. If we are in one of those cases, make the attempt. If it
        // succeeds, return the result. If not, give a compile-time error regardless of context.
        try
        {
            newValue = FoldDecimalBinaryOperators(kind, valueLeft, valueRight);
        }
        catch (OverflowException)
        {
            Error(diagnostics, ErrorCode.ERR_DecConstError, syntax);
            return ConstantValue.Bad;
        }

        if (newValue != null)
            ` return ConstantValue.Create(newValue, resultType);

        if (CheckOverflowAtCompileTime)
        {
            try
            {
                newValue = FoldCheckedIntegralBinaryOperator(kind, valueLeft, valueRight);
            }
            catch (OverflowException)
            {
                Error(diagnostics, ErrorCode.ERR_CheckedOverflow, syntax);
                return ConstantValue.Bad;
            }
        }
        else
            ` newValue = FoldUncheckedIntegralBinaryOperator(kind, valueLeft, valueRight);

        if (newValue != null)
            ` return ConstantValue.Create(newValue, resultType);
        else 
            ` return null;
    }

There are places where I probably wouldn't write it with backticks. E.g. when the if has a block then the else should be a block as well, even if it is only one statement. But that's a matter of style guides.

@dstarkowski
Copy link

if (condition == true)
    return null;

Starts with keyword, indented properly.

if (condition == true)
    ` return null;

Starts with character that has no meaning, shifts indentation in weird way and actually requires you to type more characters. How is this better?

The reason I have already stated detailed above. In short: the coding style rule { } exists for a reason

If you don't want to use braces, maybe it's time to speak with your team and change the rule?

In C# 5 I was using braces for everything. Everything in a block looked so consistent to me. :)
But the language changed. C# 6 introduced expression bodied members and C# 7 built on it. Now those blocks look weird.

Technology changes and sometimes makes some rules outdated. If your team still chooses to stick with { } I cannot imagine they would go with what you're proposing.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

Now those blocks look weird.

If that's your opinion on expression bodies then you're anyway no candidate for these kind of additions.

If you don't want to use braces, maybe it's time to speak with your team and change the rule?

See "2. putting the same statement on the next line, with indention, is also considered bad style."
An "introduction" to the same line however will change the game.

@dstarkowski
Copy link

@lachbaer

If that's your opinion on expression bodies then you're anyway no candidate for these kind of additions.

What do you mean by my opinion on expression bodied members? I didn't write anything about them except that blocks look weird. Meaning single line if, for, etc. with { } next to expression bodied members and bunch of chained linq methods don't look as consistent as they did few C# versions ago.

is considered bad style (by me/team/company)

So as I suggested, maybe you should talk to your team and reevaluate the rules since you disagree with them.

What does your proposal really introduce? What's the difference (except of 'bad practice') between following three:

if (true)
    return null;
if (true)
    ` return null;
if (true)
    /**/return null;

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

reevaluate the rules since you disagree with them

I don't disagree with the existing rules. I.e. I agree upon having no statements following if on the same or the following line. Unless, however, the following line would somehow not start with the actual statement directly. See my explanation:

if following line starts with the actual statement:

if (true)
    return null;

Line starts with comment trivia, also very bold trivia:

if (true)
    /**/return null;

Line does not start directly with statement. This indicates as first sight - before the eyes and the brain have adapted to the indention (human factors, seriously!) and before the brain has analyzed that behind the if is really nothing else - that line belongs directly to the statement above it.

if (true)
    ` return null;

I don't really know how I should express that differently...?! 😕 If you have any other idea how the goal of omitting the braces without the points 1. and 2. from above (bad style rules), then it will be very welcome by me here.

@bondsbw
Copy link

bondsbw commented May 9, 2017

I have always written if in one of two ways:

if (true) return null;
if (true)
{
    return null;
}

But never do I write a multi-line if without braces.

It is so simple, allows me to occasionally write more succinct code, and does not violate the style rules that help prevent overlooking multiple indented lines that are not subject to the if.

@dstarkowski
Copy link

dstarkowski commented May 9, 2017

@lachbaer
Your team want's brackets. You don't want brackets. I don't see how you agree with them but that's not the point.

You basically reject 100% of existing solutions saying it's bad practice and think adding new syntax that does completely nothing is a solution. Well I consider writing additional characters that don't do anything bad practice and would forbid what you're proposing the moment new version of C# ships.

Also you still didn't explain to me how my three examples differ. Why are all 8 of your suggestions => ^ -> [] _ \ :: ` for this new do-nothing operator good and my /**/ bad? Mine is already implemented in the language.

@bondsbw
And from all three single line is the only one I don't use. Two things are happening in one line (condition and return) and I don't like that. But that proves that there is no style that everyone follows.

@bondsbw
Copy link

bondsbw commented May 9, 2017

@dstarkowski Agreed, at some point it boils down to preference and awareness.

Someone out there loves Whitesmiths style. Someone believes minification is a work of art. So long as I'm not working with those people, I'm fine. (btw I'm not comparing this suggestion to those abominations 😄)

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

But that proves that there is no style that everyone follows.

Amen (or Amin), brother 😃

It is really hard for me to explain my concerns and drawbacks with the current styles! I'll try again.

First, C, C#, D, Java, ECMAScript, PHP and probably others all have the same Non-Empty-Statement or Block-Statement syntax. From that point there is absolutely no need for a change, because it so common to everybody.

But they all suffer from the same "problem" (when seen as such). Many maybe most programmers, by team convention or personal preference, don't use the Non-Empty-Statement syntax. The first question that must be answered to understand my thoughts is why they loath it.

My believe is that one reason is what @dstarkowski just stated above: "Two things are happening in one line [...] and I don't like that".

Another is probably what we all have encountered already. When there is only one statement and you need another one you either have to create a braced block anyway or you run into semantic errors.

// Version 1.0
if (condition)
    return;

// Version 1.1 (ups, forget to block the statements)
if (condition)
    RaiseEvent();
    return;

The result is that every 'than/else/for statement' is wrapped in a block an thus creates the "excessive empty space" that leads to scrolling and a lack of overview, especially in longer methods.

If now a little syntax change is added (maybe the other languages follow in the future?) the actual reason for the guideline why everything, every single statement, is braced vanishes.

(I go with : again)

// Version 1.0
if (condition)
    : return;

// Version 1.1 (compile time error)
if (condition)
    RaiseEvent();
    : return;

// Version 1.2 (you can easily spot that something is weired)
if (condition)
    : RaiseEvent();
    return;

And even if you do put the statement in one line, you can profit from that, if you like it.

if (cond1)               :  return false;
if (condition2)          :  return true;
if (yetanothercondition) :  return null;

That has a bit the look of a switch/case statement, but only because of the left-to-right-moving-eye catcher colon :.

Another bonus, if you like, is that a custom analyzer could warn or error you if you forget to either start a block after if or use the colon.

Summary

I strongly believe that the reluctance of non-brace-blocked substatements come from the just explained reasons. Introducing an optional seperator would eliminate them and make non-braced one-statement lines a first class citizen again in the style guides.

If I still have not explained it correctly or extensively enough, then please let me know.

@lachbaer lachbaer changed the title Adding arrow expression syntax to if-else clauses Adding statement seperator to if-else, for, etc. clauses May 9, 2017
@HaloFour
Copy link
Contributor

HaloFour commented May 9, 2017

@lachbaer

You're explaining them all just fine. They're just being appropriately dismissed as entirely unnecessary and redundant. As Cyrus has explained multiple times these tiny language tweaks offer nothing that make them worth adopting. Any perceived benefit is immediately lost to the fact that it's duplicitous and would only confuse and fracture code bases, probably even causing more style arguments than it would solve.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

@HaloFour Without having it we wouldn't know. With best regards from Schroedingers cat. 🐱 Also you can call that evolution 😀

Let's assume an existing code base that follows the guide lines exceptionally and has no standalone substatements. When introducing the seperator with erroring analyzer there won't be any code break. We can start migrating, manually or with automatic code rewrites. I doubt that it will lead to confusion.

And when the analyzer is put to "warn" then any occurances of guide violating standalone substatements are reported. I still doubt that it will confuse and fracture code bases.

@bondsbw
Copy link

bondsbw commented May 9, 2017

Why not just use an indention analyzer? That way you can get rid of the style guideline and let the compiler tell you when you're wrong.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

Why not just use an indention analyzer?

I don't like snakes 🐍 😉 Indentions don't have the optical appearance that helps when overflying the code with the eyes. They are just... empty.

Oh, and with this addition were no real actual need for an analyzer.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

Maybe I could play around with one of the many indent styles or create an own one.

But it would be great if Visual Studio (Code) could help me with that by allowing custom brace styles for different constructs in the way it handles "Codestyle / Naming" in the options. Constructs being the different statements together with the information whether the block only consist of one or more statements.

Now VS always destroys my formatting attempts or I have to put the braces and indention manually for every statement construct.

Or does anyone know about an Add-In that does that already?

@jnm2
Copy link
Contributor

jnm2 commented May 9, 2017

@lachbaer See the entire list of pages underneath Tools > Options > Text Editor > C# > Code Style > Formatting, new in VS2017.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

@jnm2 Those options are limited. There is no possibility to chose between e.g. Allman, Horstmann or Pico style or even custom styles. A bit more control as there is for "Code Style > Naming" would be nice.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented May 9, 2017

@lachbaer Feel free to submit PRs over at dotnet/roslyn. But this is not relevant to dotnet/csharplang.

Note: the team is not going to invest here as there is practically no demand for things like Horstmann bracing. Seriously, we haven't gotten a single customer to ever ask for it. We're just not going to spend cycles on this sort of thing when there is actual practical work we can do that can benefit large numbers of people.

@lachbaer
Copy link
Contributor Author

lachbaer commented May 9, 2017

@CyrusNajmabadi
Instead of always blaming me with your comments, what really makes me sad and desperate somehow, do you have an actual solution? Please take into account everything that I have stated above.

And a further question. Does the produced empty space that makes one scrolling through many lines of code never bother you when re-inspecting your or third party code?

@CyrusNajmabadi
Copy link
Member

Instead of always blaming me with your comments,

I am not blaming you. I'm letting you know where the appropriate place for bugs is. And i'm telling you the team's general perspective on work like this.

do you have an actual solution?

An actual solution for what? To me i don't see a problem here. If i want non-brief code, i write things in a non-brief fashion . If i want brief code, i use the language constructs available for that.

Does the produced empty space that makes one scrolling through many lines of code never bother you when re-inspecting your or third party code?

Not really. In the rare case where it has, i've just fixed the code up.

@lachbaer lachbaer closed this as completed May 9, 2017
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

8 participants