dotnet / csharplang

The official repo for the design of the C# programming language
11.32k stars 1.02k forks source link

try/catch expression #786

Closed chuck-flowers closed 7 years ago

chuck-flowers commented 7 years ago

An interesting idea to extend the concept of the ?? operator. Often I find myself doing single statement try blocks just to have a single statement catch block to rethrow or set a default value. So I thought that a syntax such as the following would be helpful.

Current:

string foo;
try
{
   foo = StringOrException();
}

catch(Exception)
{
   foo = FOO_DEFAULT;
}

Desired:

string foo = StringOrException() !! FOO_DEFAULT;

The only drawback I see is not being able to specify the exception being caught. Not entirely sure what the cleanest way to add that to the operator is but I'm open to suggestions.

bondsbw commented 7 years ago

This could encourage use of exceptions in control flow, so I'm not very supportive of that aspect.

I feel something like it could be more compelling for logging + rethrow.

bondsbw commented 7 years ago

Maybe something like this, assuming we build on the proposal's syntax:

void ThisCanThrow()
{
    ThrowFooException() !! LogFooException;      // Logs the FooException
    ThrowRegularException() !! LogFooException;  // Does not log, because it is not a FooException
}

void ThrowFooException() => throw new FooException();
void ThrowRegularException() => throw new Exception();
void LogFooException(FooException fe) => ...

That would be roughly translated as:

void ThisCanThrow()
{
    try
    {
        ThrowFooException();
    }
    catch (FooException e)
    {
        LogFooException(e);
        throw;
    }
    try
    {
        ThrowRegularException();
    }
    catch (FooException e)
    {
        LogFooException(e);
        throw;
    }
}

The type of each catch is determined from the method parameter type, so the method must have exactly one non-default parameter. (Otherwise this syntax will need to be extended to capture the exception into some variable.)

chuck-flowers commented 7 years ago

For the rethrowing case the syntax would need to allow for a reference to the caught exception as a parameter for the new exceptions constructor.

bondsbw commented 7 years ago

(And obviously the second try/catch is dead code in my example.)

chuck-flowers commented 7 years ago

Sorry posted at the same time you did. I kinda like that syntax. Feels like an 'Exception Handler'

chuck-flowers commented 7 years ago

If the signature doesn't match do we rethrow as if there wasn't a try catch or can we string together multiple !! operators like multiple catch blocks?

chuck-flowers commented 7 years ago

Example of multiple !! operators

private void Foo()
{
   ThrowException() !! LogSpecificException !! LogException;
}

private void ThrowException => throw Exception();

private void LogSpecificException(SpecificException ex) => Log(ex);

private void LogException(Exception ex) => Log(ex);
HaloFour commented 7 years ago

I certainly don't like any syntax which involves automatically weaving in method calls based on some opaque convention. The fact that LogFooException would somehow magically get invoked only if the exception thrown is of the right type and only if you use this special syntax which would otherwise swallow and ignore the exception seems like it would make understanding the flow of the code nearly impossible.

In my opinion it is not a recommended pattern to treat exceptions in this manner. As such the language shouldn't encourage it through shorthand. You already have try/catch syntax for those (hopefully) limited situations where you need to break this out, or you can write a very simple helper method to shorten it for you:

public static class Helpers {
    public static T Try<T>(Func<T> action, T defaultValue) {
        try {
            return action();
        }
        catch {
            return defaultValue;
        }
    }
}

// later
var foo = Try(StringOrException, FOO_DEFAULT);
bondsbw commented 7 years ago

@HaloFour ThrowException() !! LogFooException never swallows the exception. It is always rethrown regardless of whether LogFooException is invoked.

It is effectively a preview mechanism.

bondsbw commented 7 years ago

@chuck-flowers

If the signature doesn't match do we rethrow as if there wasn't a try catch or can we string together multiple !! operators like multiple catch blocks?

Seems reasonable.

HaloFour commented 7 years ago

@bondsbw

I see, so providing a method group as the default value would weave that method into the exception handler? So, how does that work if the expression type is a matching lambda and a method group would actually be a valid default value on its own?

Action<FooException> var = MethodThatCouldThrow() || LogFooException;

Having said operator sometimes throw and sometimes return some value seems like it would be incredibly unintuitive.

bondsbw commented 7 years ago

@HaloFour

Sorry for the confusion. To clarify, my suggestion is a change to this proposal such that it does not use any default value. Only a method group would be allowed. (It might deserve its own separate proposal.)

Or lambdas should be fine too:

ThrowException() 
    !! (BadException e => logger.Error(e.Message))
    !! (Exception e => logger.Warn(e.Message));

Thanks for bringing that up. Lambdas would be a very straightforward way to access the exception variable.

HaloFour commented 7 years ago

@bondsbw

To clarify, my suggestion is a change to this proposal such that it does not use any default value.

In that case it seems quite unrelated to the proposal here which seems to be an "exception coalescing" operator in the same vein as ??, but for exceptions.

bondsbw commented 7 years ago

@HaloFour Right, it is inspired by the syntax but otherwise is different. I'll create a new proposal.

EDIT: See #787.

iam3yal commented 7 years ago

Often I find myself doing single statement try blocks just to have a single statement catch block to rethrow or set a default value.

Not sure how often is often but if something like this would be introduced I'd prefer to have something like the following which is closer to what we have today:

string foo = try Foo() catch(Exception) => FOO_DEFAULT;

Or multiple catch blocks:

string foo = try Foo() 
          catch(Exception) => FOO_DEFAULT
          catch(AnotherException ex) => throw new MyException(ex)
          catch(YetAnotherException) => "Hello World";

Or maybe something like this:

string foo = try Foo() 
          catch(Exception): FOO_DEFAULT
          catch(AnotherException ex): throw new MyException(ex)
          catch(YetAnotherException): "Hello World";
jnm2 commented 7 years ago

Duplicate of https://github.com/dotnet/csharplang/issues/220?

chuck-flowers commented 7 years ago

I like that but I'd also like to be able to remove the braces for statements and not just expressions. Like the way loops and if statements work now for single statement blocks. Not sure if allowing both of those would be ambiguous without modifying the syntax of one or the other.

jnm2 commented 7 years ago

See also https://github.com/dotnet/csharplang/issues/616 for the expression form without braces.

jnm2 commented 7 years ago

Partial overlap of https://github.com/dotnet/roslyn/issues/16072. Anything that promotes a catch-all I will vehemently downvote because it is so badly abused. It's rare that the only thing in a catch block should be return defaultValue or foo = defaultValue and most examples I've seen are pathological.

chuck-flowers commented 7 years ago

I suppose all possible avenues of discussion regarding this suggestion have been discussed elsewhere. I'll close it.

DavesApps commented 6 years ago

@chuck-flowers slight variation on the original idea here: https://github.com/dotnet/csharplang/issues/965

VBAndCs commented 5 years ago

I would write it as : value = try (expr1, expr1); try returns the value of expr1 if it succeeded, or retirns the value of expr2 if expr1 throws exception)