Improving the "return" keyword to do conditional early exists of the method.

[proteo5]

Background
It is very common that on a method after some logic is meet there is no longer the need to continue executing the rest of the code so is normal to do early exists a in the middle of the method.

Usually there is two approaches to handle do this.

The first one, only one exit:

//divide if even
public int DivideEven(int x)
{
    int result;
    if (x == 0){
        result = 0; // Return 0 if input is 0
    }
    else {
        if(x_is_odd_logic)
        {
            result = 0; // Return 0 if input is odd
        }
            result = x / 2; // Divide the number if it even
    }
    
    return result;
}

The problem with this approach is that the logic can became so complex that it will lead to indentation hell.

The second one, early exit:

//divide if even
public int DivideEven(int x)
{
    if (x < 0) return 0; // Return 0 if input is negative

    if(x_is_odd_logic) return 0;  // Return 0 if input is odd

    return  x / 2; // Divide the number if it even
}

This is so much cleaner and readable way, but now we have a lot of if (condition) return value; statements.
On real world scenarios there is a lot if this single line codes over the place.

Proposal
Since this is very common, this can be simplified if we can allow the return keyword to accept a condition like the if statement it like this return(condition) value; :

//divide if even
public int DivideEven(int x)
{
    return(x < 0) 0; // Return 0 if input is negative

    return(x_is_odd_logic) 0;  // Return 0 if input is odd

    return  x / 2; // Divide the number if it even
}

// general code
public void general_method()
{
    return(Logic1); 

    return(Logic2);  

    return;
}

or for clarity also can be added a new keyword like returnIf like this returnIf(condition) value;:

//divide if even
public int DivideEven(int x)
{
    returnIf(x < 0) 0; // Return 0 if input is negative

    returnIf(x_is_odd_logic) 0;  // Return 0 if input is odd

    return  x / 2; // Divide the number if it even
}

// general code
public void general_method()
{
    returnIf(Logic1); 

    returnIf(Logic2);  

    return;
}

[HaloFour]

See: #2195

[HaloFour]

That is a virtue. C# doesn't want bespoke syntax to represent every possible combination of other features.

[proteo5]

C# is not a person that want or doesn't want things.
C# is a product out of the community needs.

[HaloFour]

C# is a language that follows a philosophy, namely that smaller language features can be composed together.

Doesn't really matter, for the couple of times this exact syntax has been suggested it has always gone nowhere and this time will be no different.

[proteo5]

Well, if a lot of people is asking for it, it definitely should be considered by the language design team.

[proteo5]

Just for fun fact, on the video I posted on the other thread, the language design team do consider what other languajes are doing to consider a new feature.
https://www.youtube.com/watch?v=ArN9R_8eL-E

[tacosontitan]

This proposal doesn't really save us much and feels like another "terse for the sake of being terse" situation which I'm seldom a fan of. A lot of codebases prefer to put the executed statement on its own line:

if (x < 0)
    return;

Others like adding braces for clarity and consistency, and I see nothing wrong with either option as opposed to something that saves a couple whitspace characters and maybe the word if and some parentheses.

[proteo5]

I guess there is some place, which I didn't find, where this is been discus by the language team.
Can you point me where can I read about it?

something like this https://www.youtube.com/watch?v=ArN9R_8eL-E

[HaloFour]

#4711 (comment)

[CyrusNajmabadi]

@proteo5 Thanks for the idea. If a lang team member decides this is worthwhile, we'll move it to the 'championed' stage. :)

[proteo5]

Thanks so much sir, for your consideration.
I can understand that this return (condition) value; doesn't seems a lot of improvement over the regular if (condition) return value;

But it seems that for a lot of us, the developers on the field, are asking for this since for us it really matter to do a thing like this.

[CyrusNajmabadi]

But it seems that for a lot of us, the developers on the field, are asking for this

I'm not really seeing that. It seems like there's been a small amount of interest/requests on this. We've looked at this several times (and continue to look when we evaluate the backlog of requests). But nothing really has changed that has made the design team currently interested in picking that up. If that changes, we will be sure to update these issues accordingly. Thanks :)

[theunrepentantgeek]

When this has been suggested in the past (see #1397 #138 among others) the syntax has been:

return value if condition;

Which requires no new keywords and would be (nearly) instantly understood by anyone encountering the construct for the first time.

The proposed form

return (condition) value;

would not only be a brand new and confusing syntax, it's ambiguous with existing code.

To illustrate, what does this mean?

var result = /* ... elided ... */
return (Parsable) result;

Is Parsable a property, variable or constant of type bool, making this an early return guard condition?

Or is Parsable an in-scope Type, making this an unconditional return of result after applying a type cast?

[Remember that whitespace has (nearly) no meaning in C#, so return(Parsable) result and return (Parsable)result must have identical meaning to the code above.]

[HaloFour]

How would the suggestion make that any shorter or more readable though? You're just trading the order of the keywords:

// existing
if (rul1.State == ResultStates.unsuccess) return rul1;
// proposed
return rul1 if (rul1.State == ResultStates.unsuccess);

Even if you could find a syntax that could spare a keystroke or two, how is that an improvement?

[proteo5]

@HaloFour well sir, like I say, a lot of the times the proposals come from a need that we have.
@theunrepentantgeek show me in an excellent way why is a bad idea and after his explanation I now agree that is not good approach.

So now I am trying to explain the issue I am facing on the field hopping there is another way or another suggestion.

#7927 (reply in thread)

[HaloFour]

IMO, the majority of the code in those statements ends up being the conditional expression and the return value expression, neither of which can be eliminated. So you're down to making either/both the return or if keywords shorter, or finding a way to come up with a weird combination of the two. But even at that extreme, you're barely moving the needle in terms of total amount of code.

If exceptions aren't unpalatable in this scenario they would make for an easier way to express an early exit due to rule violation:

public class RuleViolationException : Exception {
    public Rule Rule { get; }
    public RuleViolationException(Rule rule) {
        this.Rule = rule;
    }
}

public static class RuleExtensions {
    public static void Assert(this Rule rule) {
        if (rule.State == ResultStates.unsuccess) throw new RuleViolationException(rule);
    }

    public static void CheckRule(this bool expression
}

// then
var rul1 = (userResult.State == ResultStates.unsuccess).CheckRule("..", "..");
rul1.Assert();

// or
public static class RuleExceptions {
    public static void CheckRuleOrThrow(this bool condition, string code, string message) {
        if (!condition) throw new RuleViolationException(new Rule(code, message));
    }
}

// then
(userResult.State == ResultStates.unsuccess).CheckRuleOrThrow("..", "..");

Could even build a DSL that would allow a fluent validation of multiple rules, something like:

Rules.Validate(userResult.State == ResultStates.unsuccess, "..", "..")
    .And(user.Status == UserStatus.active.ToString(), "..", "..")
    .And(etc);

There are probably a couple of other different approaches you could take here if this rule validation ends up getting tedious.

[theunrepentantgeek]

I don't think a language feature is what you need here - no matter the syntax used, you'd still have all those separate guard clauses up front cluttering the flow for anyone reading the method.

At the risk of veering off topic, one approach (of many) to simplify your code would be this:

First, modify CheckRule to implement the four operators & (and) | (or) true and false, allowing you to use && and || to combine the rules.

Second, move the actual guard predicates into well named intention-revealing private methods.

And then you could write your code like this:

var validation = 
    CheckRule("USCE10", "El usario no existe.", UserExists(userResult))
	&& CheckRule("USRE20", "El usuario no existe", UserIsActive(user))
	&& CheckRule("USRE30", "El usuario no existe.", RequestHasExpectedRecoveryToken(user, req))
	&& CheckRule("USRE40", "el token ha expirado.", RecoveryTokenHasNotExpired(user))
	
if (validation.State == ResultsStates.unsuccess) 
{
    return validation
}

Even though there is only one early exit point, rule evaluation stops as soon as you have a failure, thanks to the way you implement the operators. (I've copied your existing vocabulary where it exists, but have added new things in English, I hope this works for you.)

[proteo5]

@HaloFour Sir, I own you a beer.

Your approach doesn't apply 100% since on the complex code there is a lot of regular "if" statements and calls to the database in between the rules checks. So the fluent validation is out of the scope.

I usually try to avoid throwing exceptions as a way to control the flow of the code because I have seen a lot of people ending with business code on the catch block, so I like to always have control of the execution flow.
Also I try to avoid the only one exit approach that I describe on the description of the post.

But you give me an excellent approach doing a combination by using and catching custom exceptions and one exit.

Let me refactor the code and se the results.

Thanks so much for your patience and your time sir.

[proteo5]

After discussing with @HaloFour , @theunrepentantgeek and @CyrusNajmabadi on how to actually solve the issue that I want to solve, this approach wasn't what I want, but actually I think is better.

I use a result class which you can find here with all the needed code https://gist.github.com/proteo5/cf629c7f4575b4ec486cb36425be270a

so first I define a custom exception:

public class CheckRuleException : Exception
{
    public readonly Result _result;

    public CheckRuleException(string message, Result result) : base(message)
    {
        _result = result;
    }

    public Result Result { get { return _result; } }
}

Then the rule will throw that exception whereever a rule doesn't pass

public static class CheckRules
{
    public static Result CheckRule(this bool rule, string Code, string Message)
    {
        Result result = new Result();
        if (rule)
        {
            result.State = ResultsStates.success;
            return result;
        }
        else
        {
            result.Code = Code;
            result.Message = Message;
            result.State = ResultsStates.unsuccess;
            throw new CheckRuleException("Error en la regla", result);
        }
    }   
}

So the final code end up like this:

public async Task<Result> Handle(UsersCForgotPasswordConfirmationI req)
{
    try
    {
        //Clean Input Data region...
        //Validate Input Dataregion...
        
        #region Business Rules
        //Verify if user exist
        var userResult = await new UsersQUser().Handle(new UsersQUserI() { User = req.User });
        var user = userResult.Data;

        var rul1 = (userResult.State == ResultsStates.success)
            .CheckRule("USCE10", "El usuario no existe.");

        //Check if user is active
        var rul2 = (user.Status == UsersStatus.active.ToString())
           .CheckRule("USRE20", "El usuario no existe.");

        //Verify if token match
        var rul3 = (user.RecoverPasswordToken == req.Token)
           .CheckRule("USRE30", "El usuario no existe.");

        //verify if token haven't expiere
        if ((!user.RecoverPasswordDateTime.HasValue) ||
                       user.RecoverPasswordDateTime < DateTime.Now)
        {
            user.RecoverPasswordToken = "";
            user.RecoverPasswordDateTime = null;
            await new UsersDUpd().Handle(new UsersDUpdI() { User = user });
            (false).CheckRule("USRE40", "El token ha expirado.");
        }
        #endregion

        //Scenarios Regions
    }
    catch (CheckRuleException ex)
    {
        return ex.Result;
    }
        catch (Exception ex)
    {
        return new Result
        {
            State = ResultsStates.error,
            Code = "UFPCE",
            Message = "Error al confirmar la contraseña.",
        };
    }
}

In this way, when a rule is not fulfill it will throw a custom exception and it will be handle on the catch (CheckRuleException ex) block properly and with that we avoid using if(condition) return value;
At least for me this solve in a better way the duplicate code I was having, and give me a way more possibilities like doing a log in an easy way among other possibilities.

Once again, thanks a lot for the feedback.