Discussion: Conditional branches, return when (...) with (...)

[lachbaer]

Idea

Many consider it a bad habit to break the code flow with jump instructions like return, break and continue.
One valid reason is, that the points of the jump are easily overseen.

A very common scenario are conditional returns, one e.g. being the check for null.

I want to discuss a syntax, that would allow for a cleaner appearance of the conditional 'jumps'.

See the following (made-up) code example:

public static IEnumerable<T> GetValueTypeItems<T>(
  IList<T?> collection, params int[] indexes)
  where T : struct
{
  if (collection == null)
  {
    yield break;
  }
  
  foreach (int index in indexes)
  {
    T? item = collection[index];
    if (item != null) yield return (T)item;
  }
}

With a syntax like

• return when (condition) with (return-value)

the above code could be written like

public static IEnumerable<T> GetValueTypeItems<T>(
  IList<T?> collection, params int[] indexes)
  where T : struct
{
  yield break when (collection == null);
  
  foreach (int index in indexes)
  {
    T? item = collection[index];
    yield return when (item != null)
                 with ((T)item);
  }
}

when and with can be interchanged.

    yield return with ((T)item)
                 when (item != null);

Syntax

• The paranthesis after the when and with expressions are mandatory for the lexer and parser.
• In case no return value is needed, like in void methods or the yield break above, the with part must be ommited
• You should also be able to return with (default) when (...), where default is the implied default of the methods return type.
• Like with traditional if clauses, a (block-) statement can be followed after with or when. This block/statement is executed before the branch/return.
• Any locals declared in the when portion are visible within that block statement.

I chose when and with, because they are more contextual than if. with sounds quite verbose to me and does in this context not conflict with the with for record types.

Practical example

In theory the idea might look unnecessary. But lease look at the following code, copied from https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs#L1415

I have rewritten that function (below) using the proposed syntax. I think that the expression of the code is much better there!

private ConstantValue FoldBinaryOperator(
    CSharpSyntaxNode syntax,
    BinaryOperatorKind kind,
    BoundExpression left,
    BoundExpression right,
    SpecialType resultType,
    DiagnosticBag diagnostics,
    ref int compoundStringLength)
{
    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);
    }

    return null;
}

Rewritten with return when () while ():

private ConstantValue FoldBinaryOperator(
    CSharpSyntaxNode syntax,
    BinaryOperatorKind kind,
    BoundExpression left,
    BoundExpression right,
    SpecialType resultType,
    DiagnosticBag diagnostics,
    ref int compoundStringLength)
{
    Debug.Assert(left != null);
    Debug.Assert(right != null);

    return when (left.HasAnyErrors || right.HasAnyErrors) with (null);

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

    var valueLeft = left.ConstantValue;
    var valueRight = right.ConstantValue;
    return when (valueLeft == null || valueRight == null) with (default);

    return when (valueLeft.IsBad || valueRight.IsBad) with (ConstantValue.Bad);

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

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

    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);
    return when (newValue != null)
           with (ConstantValue.Create(newValue, resultType));

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

    // 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;
    }

    return with (ConstantValue.Create(newValue, resultType))
           when (newValue != null);

    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);
    }

    return when (newValue != null)
           with (ConstantValue.Create(newValue, resultType));

    return null;
}

[HaloFour]

You aren't eliminating any jumps, you're just disguising them. The control flow logic is exactly the same.

[lachbaer]

@HaloFour

You aren't eliminating any jumps, you're just disguising them

It's not my intent to eleminate them. And I would rather say that this syntax dis- disguises the jumps!

And to me, I love the appearance of

    return when (valueLeft == null || valueRight == null) 
           with (default);
    return when (valueLeft.IsBad || valueRight.IsBad)
           with (ConstantValue.Bad);
    return when (kind.IsEnum() && !kind.IsLifted())
           with (FoldEnumBinaryOperator(syntax, kind, left, right, diagnostics));

    return when (IsDivisionByZero(kind, valueRight)) with (ConstantValue.Bad)
        Error(diagnostics, ErrorCode.ERR_IntDivByZero, syntax);

or

    return with (default)
           when (valueLeft == null || valueRight == null);
    return with (ConstantValue.Bad)
           when (valueLeft.IsBad || valueRight.IsBad);
    return with (FoldEnumBinaryOperator(syntax, kind, left, right, diagnostics))
           when (kind.IsEnum() && !kind.IsLifted());

    return with (ConstantValue.Bad) when (IsDivisionByZero(kind, valueRight))
        Error(diagnostics, ErrorCode.ERR_IntDivByZero, syntax);

instead of

    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);
    }

    if (IsDivisionByZero(kind, valueRight))
    {
        Error(diagnostics, ErrorCode.ERR_IntDivByZero, syntax);
        return ConstantValue.Bad;
    }

[HaloFour]

I don't. It's a lot of extra syntax for zero benefit. You can already achieve a terser form by simply omitting the blocks:

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);

if (IsDivisionByZero(kind, valueRight)) {
    Error(diagnostics, ErrorCode.ERR_IntDivByZero, syntax);
    return ConstantValue.Bad;
}

Your last "statement" doesn't even make sense as you're implying that some kind of embedded-statement or block can follow the return? Your code jumps around more than the code with actual jumps.

[DavidArno]

If you reduced down the syntax to:

return <value> when <guard expression>;

then this proposal is effectively adding when guards, which we have in catch and case statements already, to return and yield.

However, as such guards can already be expressed as if (<guard expression>) return <value>;, one has to ask: what's the point, beyond "lov[ing] the appearance" of one syntactic form over another?

[lucamorelli]

I have a similar problem while doing model validation at the beginning of method calls, when If have to add several preconditions checks.
Obviously there are a lot of possible way, but often ends with several

if ([condition1])
  return (code1);
if ([condition1])
  return (code2);
....

this lead often to have a beginning of the code that is a bit confused, long, and hard to share between method calls with similar model. One scenario is the validation of parameters of method calls with several parameters.

In this situation it's something annoying because you need at least 2/3 rows of code ofr every check.

some sample (it's depending on the type of data) could be:

if (!Preconditions.IsValidVatCode(vatCodeParam))
  return false;
if (!Preconditions.IsValidZipCode(zipCodeParam))
  return false;
if (!Preconditions.IsValidCity(cityCodeParam))
  return false;

In my case it depends of data values

[lachbaer]

@lucamorelli
In your case this could be rewritten like

return when (!Preconditions.IsValidVatCode(vatCodeParam)
          || !Preconditions.IsValidZipCode(zipCodeParam)
          || !Preconditions.IsValidCity(cityCodeParam))
       with (false);

@DavidArno

what's the point, [...] of one syntactic form over another

THE point is actually to put the relevant code part - namely the branch/return - at the beginning of the code line!

See, I'm talking about the looks now:

if (obj == null) return;
// or
if (list.Count == 0) return null;

This primarily looks like an if clause that, when hiding the following statement, looks as if it is part of the calculatoric algorithm. Following the if can be anything, a variable definition, a method call, etc.

return when (obj == null);
// or
return with (null) when (list.Count == 0);

This is actually the same statement, but this time you see immediately that the whole line is about the branching/returning. The focus is shift towards then return! Same for

if (list.Count == 0) {
    Console.WriteLine("List is empty, nothing to be done...");
    return null;
}

Here again, the purpose of the whole if-block is to return with a log message. But on the first sight it does not really distinguish from any other if that is used for the actual calculations the method is doing. The real purpose is more visible from this:

return when (list.Count == 0) with (null) {
    Console.WriteLine("List is empty, nothing to be done...");
}    

(no, it's not really shorter, but that's not the idea anyway).
The intention of the programmer here is to return at this point - and just issue a log message additionally.

Of course you can violate this construct to irritate the reader. But you could do that with other constructs as well, so this doesn't count as an argument.

Besides, what is the purpose of LINQ queries? Every query can be expressed as good old plain C# method calls. Null-respecting operators don't add real value either. They, too, can be expressed in two lines of code.
[I don't mean that for real!]

This whole idea is not about creating new ways, it is about making the expression, the real purpose of that lines of code much more expressive - that's all.

@HaloFour

return with (ConstantValue.Bad) when (IsDivisionByZero(kind, valueRight))
     Error(diagnostics, ErrorCode.ERR_IntDivByZero, syntax);

Your last "statement" doesn't even make sense as you're implying that some kind of embedded-statement or block can follow the return? Your code jumps around more than the code with actual jumps.

Yes, it absolutely makes sense. Then you haven't understood the meaning of this idea - sorry!
I wouldn't go for the way to write the statement without braces. I just did it that one time to show that it could also be done that way, like with standard if-clauses.

The purpose of allowing statement(s) behind the return when(...) is to "unwind" the return statement from the inner block, so that anyone can see at once, that the only purpose of this block is to return (or break, yield, goto, maybe even continue).

[HaloFour]

@lachbaer

Yes, it absolutely makes sense. Then you haven't understood the meaning of this idea - sorry!

No, I got it. It just doesn't make sense. The idea that this "construct" can be followed by an embedded-statement which is executed and then it performs the return is crazy. It completely violates the expected flow to control code in C# (or frankly any imperative programming language).

There's a reason you write if before return, the if happens first, and is more important.

[jnm2]

@lachbaer

return when (!Preconditions.IsValidVatCode(vatCodeParam)
          || !Preconditions.IsValidZipCode(zipCodeParam)
          || !Preconditions.IsValidCity(cityCodeParam))
       with (false);

I think this looks cleaner:

if (!Preconditions.IsValidVatCode(vatCodeParam)
 || !Preconditions.IsValidZipCode(zipCodeParam)
 || !Preconditions.IsValidCity(cityCodeParam))
{
    return false;
}

[lucamorelli]

@jnm2 it depends on data. In this example could work becase you have the same return type, but if you have

if (!Preconditions.IsValidVatCode(vatCodeParam))
return Error.Vat;
if (!Preconditions.IsValidZipCode(zipCodeParam))
return Error.Zip
if (!Preconditions.IsValidCity(cityCodeParam))
return Error.City;

this doesn't work

[lachbaer]

@HaloFour

the if happens first, and is more important.

I totally do not agree on being more important! Different people, different culture, different view...

It completely violates the expected flow to control code in C#

A bit of, yes. But I didn't see any other possibility to put the return at the beginning of the construct and still having the opportunity to execute some side-code.

[lachbaer]

@lucamorelli

return when (!Preconditions.IsValidVatCode(vatCodeParam)) with (Error.Vat);
return when (!Preconditions.IsValidZipCode(zipCodeParam)) with (Error.Zip);
return when (!Preconditions.IsValidCity(cityCodeParam)) with (Error.City);

PS: you can highlight the code by using ```cs at the beginning instead of just ```.

[HaloFour]

@lucamorelli

It works just fine:

if (!Preconditions.IsValidVatCode(vatCodeParam)) return Error.Vat;
if (!Preconditions.IsValidZipCode(zipCodeParam)) return Error.Zip;
if (!Preconditions.IsValidCity(cityCodeParam)) return Error.City;

@lachbaer

I totally do not agree on being more important! Different people, different culture, different view...

C# is a programming language. It's syntax and order has already been decided. To introduce more dialects just to make it more comfortable to non-English speakers would only make it unnecessarily complicated.

A bit of, yes. But I didn't see any other possibility to put the return at the beginning of the construct and still having the opportunity to execute some side-code.

There's a reason for this.

[lucamorelli]

@HaloFour I know, just I don't like write code this way because to keep everything in one line you can have a series of rows with different length. This Thing that make harder to find the main parts of the control while reading. Even worst is the idea of align all the returns to the same column.

[lachbaer]

@HaloFour
Are you reading from left-to-right? I guess so. 😉
Close your right eye (symbolically speaking) and tell me what is going to happen:

if (!Preconditions.IsValidVatCode(vatCodeParam)) [...can't see this]
if (!Preconditions.IsValidZipCode(zipCodeParam)) [...can't see this]
if (!Preconditions.IsValidCity(cityCodeParam)) [...can't see this]

What is going to happen...?

Now again:

return when (!Preconditions.IsValidVatCode(vatCodeParam)) [...can't see this]
return when (!Preconditions.IsValidZipCode(zipCodeParam)) [...can't see this]
return when (!Preconditions.IsValidCity(cityCodeParam)) [...can't see this]

And again:

return with(Error.Vat) [...can't see this]
return with(Error.Zip) [...can't see this]
return with(Error.City) [...can't see this]

Because of return with you know a mandatory condition is comming.
Because of return when you know a return value must come (when method isn't void).

Also, return when gives you both, the essence of the condition and the condition at the beginning:

  // oh, an `if`. Must be of importance to the algorithm
if (!Preconditions.IsValidVatCode(vatCodeParam)
  // where is the statement...? Ah, another condition is coming:
 || !Preconditions.IsValidZipCode(zipCodeParam)
  // but now I expect to see what `if` is doing - ah, no, another condition...
 || !Preconditions.IsValidCity(cityCodeParam))
  // certainly there is a further condition here?
  // Oh yes, a block begins, but now I'm already too tired to keep reading,
  // I'm more interested in the actual algorithm
{
    return false;
}

// vs.
return with (false) // oh, this is only a "jump out" - not interested in reading now, 
                   // I'm skipping this part for now
       when (!Preconditions.IsValidVatCode(vatCodeParam) ...

In case you are not interested in premature returns (as a reader) the algorithms are more easily to see for you.

Same, when you are interested in the returns.

if (!Preconditions.IsValidVatCode(vatCodeParam))
{
    return false;
}

What's your reading flow now? You will search for the return, and then go upwards to see the condition. The reading "completely violates the expected flow to control code in C#" 😉

[HaloFour]

@lachbaer

Close your right eye (symbolically speaking) and tell me what is going to happen

Tells me that nothing is going to happen unless the condition is met. That's more important than what might actually happen.

Solves no new problems. More verbose than existing solution. Completely redundant.

[lachbaer]

@HaloFour
I think we both will not agree upon this finally.
But I would be happy if you had some constructive arguments that will make me think. I think I have already given enough.

[jnm2]

@lachbaer This doesn't do much to make returns easier to find because it only handles the case of non-nested if statements. There are many other ways to nest return statements, such as switches and loops and try blocks. It won't stop us from having to scan the whole method to find all the exit points. What does help with that is the IDE. Put your cursor on one and it highlights all the exit points.

[lachbaer]

@jnm2 I also (somewhere) proposed to highlight branching keywords differently in the IDE, e.g. violet instead of standard blue.
No, it doesn't solve the problem completely. But when you look at your code, I bet the most occurences of intra-method returns would profit in readability. Just look at the Roslyn code in the very first post.

@HaloFour

To introduce more dialects just to make it more comfortable to non-English speakers would only make it unnecessarily complicated.

Yes, please tell that to the translators of Visual Studio! 🤣 I always install English IDEs, because the German translations make me sick! 🤒 But then, what is the point of having translations anyway...?! Sorry to say, but I think that your opinion on that is a bit illiberal. In an open world a modern programming language should be open to other cultures as well, as long as it stays understandable for all cultures.

[HaloFour]

@lachbaer

In an open world a modern programming language should be open to other cultures as well, as long as it stays understandable for all cultures.

A programming language should have simple concise rules and, preferably, exactly one way to accomplish something. Additional ways should only be introduced if they provide a significant benefit. Programming languages should not bother to approach the huge variety of nuances and grammatical differences between the litany of possible spoken languages. Attempting to chase dialects, SVO, conjugation, alternate vocabularies, etc. would only result in making the language near impossible to learn and understand.

[lachbaer]

@HaloFour

Additional ways should only be introduced if they provide a significant benefit.

And that is what I see in this proposal. We can argue about the syntax or the one or other point, but not on the issue of putting branching keywords at the beginning of a statement.

As just a quick idea, an intermediate way could be to not using the return keyword as the first one.

when (!Preconditions.IsValidVatCode(vatCodeParam))
    return Err.VAT;

It is actually equivalent to if with two major differences:

1. when has that different appearance as a keyword, introducing a branch
2. the following statement (or block) must end with a branch

Especially 1. is important to me! (And it needn't to be when, e.g. could be a new branch.)

When you work in a team and have others read your code, e.g. in an open source project, or when you return to your own code after a while, don't you think that there are enough premature returns that can be seen easier with a different construct? Have a thoughtful look at your code. 😃

[HaloFour]

@lachbaer

Solves no new problems. More verbose than existing solution. Completely redundant.

This proposal provides no benefits except to make you feel better about the order of grammar in your code. If you really want it I suggest forking C# and making your own language.

When you work in a team and have others read your code, e.g. in an open source project, or when you return to your own code after a while, don't you think that ...

I work on a team of well over several hundred developers, within an organization of several dozen teams. We maintain our own products as well as dozens of open source projects. What I like, above all, is consistency. One way to do something. Not 50 based on subjective ideas of in which order grammatical structures should happen to appear. One syntax to rule them all and in the language specification bind them.

[lachbaer]

@HaloFour

If you really want it I suggest forking C# and making your own language

That's gross! 😞

I work on a team of well over several hundred developers, within an organization of several dozen teams.

And that is what is giving you probably blinkers, no offense!

What I like, above all, is consistency. One way to do something

That's what guidelines are for, no matter what structures the language offers.

Just imagine for a second - besides all current ways - that since years in all your company's code all conditional returns had the appearance of this proposal, together with the guideline to strictly use it when (conditionally) jumping out of a method prematurely. How would your code look like then? Wouldn't you already have gotten used to it? Maybe even meanwhile love that construct, because it gives the consistency you long for?

[HaloFour]

@lachbaer

That's an irrelevant hypothetical. This proposal isn't about whether or not to use an existing redundant language feature. It's about allocating the resources to modify the compiler and language specification in order to incorporate a new redundant language feature.

If C# always had a completely different syntax for performing conditionals and/or breaking from the current method than I would argue against adding another syntax, even if the proposed syntax was the current syntax. Other languages have their own syntax, and I don't/won't argue that they should incorporate the syntax from C#. If they want to waste their time doing so that's their business.

Do you also argue that C# should adopt VSO syntax? Let's stick the method before the instance. Some language families work like that. Sometimes the O comes first, so let's stick the arguments before the method. In some languages negation is a post-fix modifier, so let's support ! as a suffix operator. Why not make every keyword translatable? Use whatever brackets are easiest for whichever keyboard we're dealing with at the moment?

Programming languages aren't supposed to be these things. I pity the programming language that tries.

[lachbaer]

@HaloFour Now you are overreacting. What a pity, I thought we could have a productive discussion here.

I never ever went the way to support multiple cultures in whatever way. But at least they must be respected somehow when taking about changes, which so ever.

The conditional returns are one of the most used patterns. The argumentation against premature returns is just theoretical, because in C# at least that is common practice all over the place, and though it never ends because of the importance of the concept. The appearance of that "special" construct, that does not differ from any other ordinary if, alone should be argument enough to undergo a review.

[DavidArno]

@lachbaer,

Serious question here: have you tried coding using ruby? The mass of proposals that you have created around if, null and expression order are all - as far as I can remember - addressed by that language. Give it a try; you might love it.

[HaloFour]

@lachbaer

But at least they must be respected somehow when taking about changes, which so ever.

The language should remain consistent with itself above all. What other cultures do doesn't matter; the C# culture matters.

The conditional returns are one of the most used patterns.

Yes it is.

The argumentation against premature returns is just theoretical, because in C# at least that is common practice all over the place, and though it never ends because of the importance of the concept.

I'm not sure why you bring this up. The people making such arguments would be just as opposed to your syntax as it is the fact that the control flow exits the method early. They're also generally opposed to break and continue within loops, throwing exceptions, etc.

For the rest of us that have no problems exiting methods early where warranted, we already have perfectly good syntax to do so. Your syntax does nothing but invert the condition and the action because you think it looks nicer that way, despite being more verbose. That's far from reason enough to bother with the time and effort required to modify the compiler and the language specification, in my opinion.

[lachbaer]

@DavidArno

have you tried coding using ruby

Actually never had a look into it yet.

@HaloFour

I'm not sure why you bring this up. The people making such arguments would be just as opposed to your syntax as it is the fact that the control flow exits the method early.

I expressed myself badly. I meant to say that despite their reasoning is understood by most programmers, nevertheless the contrary has become common practice in C style languages. One of their main arguments, as far as I remember, is that the jumps can easily be overseen. An argument I agree upon.

This proposal is an attempt to add a construct to C# that in no means introduces a new concept, but instead "reformats" a common and heavily used pattern in a way that pays its importance justice - finally in a way that fits the language of course.

That's far from reason enough to bother with the time and effort required to modify the compiler and the language specification

That's not our call to make, thankfully ;-)

[lachbaer]

@HaloFour I wonder, if the following approach suits you 😃

It makes use of attributes on statements/blocks, a non-available feature by now. But adding that would allow for many other future uses. The attributes are there for compile-time only, so no CLR change is needed.

// This is a conditional return, just ensure the following statement returns
[return: Return]
if (obj == null) return null;

// Ensure the following statement returns null
[return: Return.Null]
if (valueLeft == null || valueRight == null) return null;

// Ensures the following statement returns the type
[return: Return.Type(typeof(ErrorEnum))]
if (!Preconditions.IsValidVatCode(vatCodeParam)) return Error.Vat;

// Ensures the following statement returns the value
[return: Return.Value(0)]
if (!Preconditions.IsValidVatCode(vatCodeParam)) return 0;

// The following is a "precondition validation block"
// The block itself __must not__ have *direct* return statement
// but in case of returns, the corresponding type must be met
[return: Return.NoDirectReturn, Return.Type(typeof(ErrorEnum))]
{
    if (!Preconditions.IsValidVatCode(vatCodeParam)) return Error.Vat;
    if (!Preconditions.IsValidZipCode(zipCodeParam)) return Error.Zip;
    if (!Preconditions.IsValidCity(cityCodeParam)) return Error.City;
}

// the following switch must return, and it must not return "null"
[return: Return.NotNullOrDefault]
switch (index) {
    case 1: return object1; break;
    case 2: return object2; break;
    default: return defaultobject; break;
}

// this is just a hint, compiler issues error if no return is found at all
[return: Return.HasReturn]
switch (index) {
    case 1: return object1; break;
    case 2: return object2; break;
    default: continue; break;
}

The used attribute classes (e.g. Return.ValueAttribute) would be nested within the System.ReturnAttribute class.

The [return: ...] target can be reused for this purpose (to make the construct stand out more, maybe it even helps the compiler).

This approach would actually serve the same purpose of this proposal in a really typical C# style. Additionally it would add compile-time-attributes on statements/blocks, that can be used in many other upcoming scenarios.

A further advantage of this syntax is that it can loosen the guideline of always surrounding if-blocks in {}, even in case of a simple return, because the initial [return: ...] attribute is sounding enough, uncluttering the code from braces.

[CyrusNajmabadi]

With the analyzer approach you have full flexibility to define and dictate 100% of the sorts of things you think should be called out ahead-of-time with all your code. As you discover new and interesting things you want to avoid/ban, you can add them. If people find these valuable for their own projects they can get your analyzer and use it themselves.

[lachbaer]

I'm really struggling to see the value here of that. If you want a test, just write a test. It sounds like you want some sort of contract feature, but that you want that contract to literally apply to each and every statement.

😢 😭 What am I doing wrong that I am misunderstood all the time.... ?!?!?!?! 😭

What sort of 'code analysis' on the return do you want to have?

Question: how would you, Cyrus Najmabadi, ensure with a test or code analysis, that a (long) if block with further nested statements always returns from the method?
And don't tell me to outsource the contents of the if to a further method! In Roslyn I see long if blocks everywhere - I don't condemn that, but it simply shows that the real world is bit different from the raw theory.

[lachbaer]

Great, now i have a lot of up front information about the 'if' :) Now what?

You are exaggerating now! This is for the concrete case of returns only...

[lachbaer]

how would you ensure with a test or code analysis, that a (long) if block with further nested statements always returns from the method?

Okay, I see, by using a comment.

[lachbaer]

I think you and i have different definitions of the word "Hidden". :)

And here we go again => human factors 😄

[CyrusNajmabadi]

What am I doing wrong that I am misunderstood all the time

You're asking for things without giving clear explanations for why they would be valuable, or why I would want to use them in my own code :)

ensure with a test or code analysis, that a (long) if block with further nested statements always returns from the method?

You've lost me already. Why would i want to test for this or analyze this? I don't approach problems going: "i need to ensure that this if statement must return."

That's not how I code, nor does it even make sense to me as something i would care about. What i care about is what funcitonality a 'method/class/module/component' provides, and testing that it does so properly. Asking how i would test if there is a return is like asking me to test a component by validating that it is implemented under the covers with lambdas. I simply don't care. It's a not a relevant or appropriate question that even comes into my mind.

And don't tell me to outsource the contents of the if to a further method! In Roslyn I see long if blocks everywhere

I don't understand. Roslyn does what it does because we don't see a problem here. If you have a problem with this style of coding you are free to code differently (for example, the way i mentioned). We don't do that ourselves because we're not the ones complaining about this sort of thing :)

[CyrusNajmabadi]

You are exaggerate now! This is for the concrete case of returns only...

Why are 'returns' any more important or interesting than anything else i mentioned?

[CyrusNajmabadi]

And here we go again => human factors

Ok. So again, why would we just do this for 'returns'. When the next person comes along and says "i want to know if other code jumps into this if block, give me an attribute for that", what do we say? Or "does ths code allocate?" or "does this code call out to other methods?" etc. etc.

You've basically said "this is a problem that i have, provide me a solution to for exactly that issue". But that's not how we do language design. Our language features need to be either:

1. broadly applicable. i.e. a huge amount of our user base will want this and benefit from it.
2. hugely beneficial. i.e. it might be a smaller group (but still need to be a lot of people), but the gain they get is enormous.

If you propose a feature which is not only applicable to just you (and maybe a handful of other people), and provides little benefit (i.e. tells you tehre is a return, when you can see the return 3 lines lower)... then that's not going ot be something anyone is champing at to get in the language.

[lachbaer]

If you have a problem with this style of coding you are free to code differently

I do not have a problem. I wanted to point out that probably many others would always vote for outsourcing with many valid arguments. But in the real world other teams have other opinions. Where your team doesn't see a problem, another team does. I'm completely liberal on this point!

[CyrusNajmabadi]

Okay, I see, by using a comment.

Yes. You're effectively saying "i want to put special documentation in my code, and have it validated". You may also be saying (though i'm not sure) "i want hte absense of this documentation to be considered a problem".

Both of these goals are solved by analyzers. Write up whatever documentation you want for your code. Write whatever analyzers you want. Make it publicly available if you want. Roslyn is there for you to get this today.

[CyrusNajmabadi]

I wanted to point out that probably many others would always vote for outsourcing with many valid arguments.

I'm not sure i know what this means.

[lachbaer]

You haven't commented on my comment on [Condition.CLang] ;-)

It was just a syntax sample of statement attributes with no further meaning. Though that one could help to copy-paste legacy C-code algorithms into an unsafe block of a C# program without having to spend too much time for language agnostic adoptions (thus making the 'C' in C# a bit prouder of itself ;-)

[CyrusNajmabadi]

@lachbaer See above:

Our language features need to be either:

1. broadly applicable. i.e. a huge amount of our user base will want this and benefit from it.
2. hugely beneficial. i.e. it might be a smaller group (but still need to be a lot of people), but the gain they get is enormous.

If:

1. we had tons of people tyring to copy/paste legacy C into C# and
2. those people were persistently complaining that the differences in the language were a problem and
3. we felt we could effectively smooth things out here then

maybe we'd do something here**. In the absence of that, we're not going to take on a language feature just because it seems "nifty" :)

--

**

It's worth nothing that, at times, we have done precisely this. 'ref-returns' are an example of how we've worked to make our language work better with the patterns native systems like C use to get very low overhead, very high performance systems. We did not go for an approach where you could just 'drop in C' (that really isn't something that could even work given how C# ends up executing at the end of the day). But we took inspiration from how many native systems work, and we produced something that would provide similar benefits, while still working within the constraints of our system (i.e. where we want to ensure safety by default).

[lachbaer]

Would you just consider to validate the following argument for statement-attributes?

• Allowing attributes on statements and blocks would allow for custom code analyzers
• Analyzers don't have to look for comments, because...
• Comments can stay comments and don't serve a programmatic purpose
• Attributes underly a concrete syntax, whereas comments can have unseen typos

Whether I use it for decorating return blocks and analyze them is then up to me. But it gives all programmers/team a syntax supported pattern.

Addendum: Especially now with Roslyn allowing us to do comprehensive analysis further places for attributes would be nice

[CyrusNajmabadi]

Proposal for Attributes-Everywhere is here: https://github.com/dotnet/roslyn/issues/6671

[lachbaer]

No place yet in dotnet/csharplang?

[CyrusNajmabadi]

Doesn't look like anyone has pushed on it to go anywhere.

[lachbaer]

But the soup of ifnot and until is not eaten yet! 😉

Our language features need to be either:

1. broadly applicable. i.e. a huge amount of our user base will want this and benefit from it.
2. hugely beneficial. i.e. it might be a smaller group (but still need to be a lot of people), but the gain they get is enormous.

Whereas for ifnot we can still argue a lot, until seems SO intuitive to me, much better than leading new programmers to while (!...). And all experienced developers will intuitively know what it means.
Again that is human factors, with human meaning a broad mass of existing people - not just me ;-)

[lachbaer]

Doesn't look like anyone has pushed on it to go anywhere

Would you mind to do so? Now that it came up here.

[CyrusNajmabadi]

I am personally not interested in championing that feature. Sorry :). You can check with StephenToub though on that bug that i linked to.

[lachbaer]

As I will close the topic anyhow shortly, Cyrus, have you already started on the property-scoped fields? I tried to start on semi-auto-properties with the field keyword and thought that a new PropertyScopeBinder could be helpful here.

[CyrusNajmabadi]

@lachbaer I put in some effort a while back, but ran into major 'scope' issues. Specifically that the size of the feature kept growing larger and larger. For example, once i started into it it became clear that while fields were a nice start, some people would then want a shared method that only the accessors could use. And then there were cases where you'd want to be able to access the fields/other-members from derived types, etc.

The scope grew so large that i had to put it on hold while i focused on other things.

[lachbaer]

@CyrusNajmabadi Complex topic! My opinion is to keep it all KISS, meaning that everything declared within a property is by definition private. For anything more complex the programmer should fall back to 'good old C#' 😃 Property-local functions are a logical "MUST" now that local functions are available. But those, too, should just be private.

[CyrusNajmabadi]

That's definitely an approach we could go :)

[lachbaer]

Though this proposal has a very broad agreement 😁 I'm closing this issue.

The best alternative (to me) are code analyzers that operate on attributes.