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

try/catch expression #786

Closed
chuck-flowers opened this issue Aug 3, 2017 · 22 comments
Closed

try/catch expression #786

chuck-flowers opened this issue Aug 3, 2017 · 22 comments

Comments

@chuck-flowers
Copy link

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
Copy link

bondsbw commented Aug 3, 2017

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
Copy link

bondsbw commented Aug 3, 2017

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
Copy link
Author

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
Copy link

bondsbw commented Aug 3, 2017

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

@chuck-flowers
Copy link
Author

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

@chuck-flowers
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

HaloFour commented Aug 3, 2017

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
Copy link

bondsbw commented Aug 3, 2017

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

It is effectively a preview mechanism.

@bondsbw
Copy link

bondsbw commented Aug 3, 2017

@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
Copy link
Contributor

HaloFour commented Aug 3, 2017

@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
Copy link

bondsbw commented Aug 3, 2017

@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
Copy link
Contributor

HaloFour commented Aug 3, 2017

@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
Copy link

bondsbw commented Aug 3, 2017

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

EDIT: See #787.

@iam3yal
Copy link
Contributor

iam3yal commented Aug 3, 2017

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
Copy link
Contributor

jnm2 commented Aug 3, 2017

Duplicate of #220?

@chuck-flowers
Copy link
Author

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
Copy link
Contributor

jnm2 commented Aug 3, 2017

See also #616 for the expression form without braces.

@jnm2
Copy link
Contributor

jnm2 commented Aug 3, 2017

Partial overlap of dotnet/roslyn#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
Copy link
Author

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

@DavesApps
Copy link

@chuck-flowers slight variation on the original idea here: #965

@VBAndCs
Copy link

VBAndCs commented Mar 12, 2019

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)

@dotnet dotnet locked as resolved and limited conversation to collaborators Mar 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants