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

[Good Proposal]™ Expression-bodied try catch like get set #1567

Closed
dangi12012 opened this issue May 26, 2018 · 48 comments
Closed

[Good Proposal]™ Expression-bodied try catch like get set #1567

dangi12012 opened this issue May 26, 2018 · 48 comments

Comments

@dangi12012
Copy link

dangi12012 commented May 26, 2018

This would make Try Catch MUCH less verbose to what it is now:
Since C# 6.0 we can have simple Lambda properties:

string name;
public string Name
{
      get => name;
      set => name = value;
}
public string FullName => $"{FirstName} {LastName}";

It would be VERY cool to have this kind of lambda available for Try Catch:

try => WebClient.DownloadFile(path);
catch(WebException ex) => Log(ex);

EDIT: This example shows the advantage much cleaner. Try the following code with current state of C#:

try
{
      try => Foo();
      catch(BarException) => return null;
}
catch(Exception e) => Log(e.Message);
finally => Free(UnmanagedResource);

If you compare how much boilerplate code was necessary for get;set before C#6.0 you can clearly see how how similar Try Catch is to that. This way code gets less verbose and more readable 👍
What do you think about Expression-bodied try catch?

@dangi12012 dangi12012 changed the title [Really cool proposal] Expression-bodied try catch like get set [Good Proposal] Expression-bodied try catch like get set May 26, 2018
@dangi12012 dangi12012 changed the title [Good Proposal] Expression-bodied try catch like get set [Good Proposal] Expression-bodied try;catch like get;set May 26, 2018
@dangi12012 dangi12012 changed the title [Good Proposal] Expression-bodied try;catch like get;set [Good Proposal] Expression-bodied try-catch like get-set May 26, 2018
@dangi12012 dangi12012 changed the title [Good Proposal] Expression-bodied try-catch like get-set [Good Proposal] Expression-bodied try catch like get set May 26, 2018
@theunrepentantgeek
Copy link

theunrepentantgeek commented May 26, 2018

The semicolon (;) usually found at the end of a statement - your suggested syntax has it showing up in the middle of the try/catch which is not only very jarring but inconsistent with the rest of the language.

Single line syntax for get/set is attractive because the implementations are very commonly single line.

In my experience, try/catch blocks are quite the opposite. The try part of the statement is typically several lines of code, and if I ever see a single line catch block during a code review it's almost always a sign that something important has been omitted.

I'm pretty sure this has been suggested before, but couldn't find it with a brief search.

[Aside: I believe the community convention is to use the tag [Proposal] only. If you didn't think it was a good idea, you wouldn't be posting it. Whether the rest of the community thinks it is a good idea is up to them.]

@HaloFour
Copy link
Contributor

@dangi12012

With braces with try/catch are required to establish to which try each catch belongs as any given try can have 0...many catch clauses. The compiler would not be able to disambiguate what the following means:

try => DoFizz();
try => DoBuzz();
catch (FooException ex) => HandleFooException(ex);
catch (BarException ex) => HandleBarException(ex);
catch (BazException ex) => HandleBazException(ex);

@MgSam
Copy link

MgSam commented May 26, 2018

You could easily make a utility method to do this for you without needing to modify the language.

@ufcpp
Copy link

ufcpp commented May 26, 2018

Good proposal but very similar to

@Joe4evr
Copy link
Contributor

Joe4evr commented May 26, 2018

[Good Proposal]

No it isn't. And please search before opening an issue, we've already seen #220, #616, #786, #1068 asking for exactly the same thing, and all of them are shut down for the exact same reason.

@gulshan
Copy link

gulshan commented May 26, 2018

I also proposed a try-catch expression at #980

@dangi12012
Copy link
Author

dangi12012 commented May 26, 2018

Well you see that people independent of each other come up with the same idea shows that there is merit in the idea.
Of course simple Lambda Try Catch cannot be nested but would make it easier for most cases. Also since it would be a new feature it could be mixed with old try/catch:

try
{
      try => Foo();
      catch(BarException) => return null;
}
catch(Exception e) => Log(e.Message);

Now try that with current state of C# and see how good it looks ;)

@theunrepentantgeek
Copy link

people independent of each other come up with the same idea shows that there is merit in the idea.

There are a lot of counter-examples that prove this is false. Take your pick.

try
{
    try
    {
        Foo();
    }
    catch(BarException)
    {
        return null;
    }
}
catch(Exception e)
{
    Log(e.Message);
}

There you go, easy. This isn't hard to read.

Of course, there is value in being brief and concise - but that has to be weighed against other constraints as well. Having ideas like this one suggested over and over again doesn't make it them good ideas, just obvious ones.

@iam3yal
Copy link
Contributor

iam3yal commented May 27, 2018

Well you see that people independent of each other come up with the same idea shows that there is merit in the idea.

Here is the thing how many times do you have to deal with exceptions yourself as opposed to write actual logic?

The only reason you get to the same idea is because it's one of the few blocks in the language that doesn't have an expression form and so asking for it is almost inevitable but it doesn't mean something should be done about it.

@Unknown6656
Copy link
Contributor

Unknown6656 commented May 27, 2018

How about the following solution (It even works since C# 3.0 !!):

public static void @try(this Action body, Action<Exception> single_catch)
{
    try
    {
        body();
    }
    catch (Exception e)
    {
        single_catch(e);
    }
}

public static void @try(this Action body, params (Predicate<Exception> condition, Action<Exception> body)[] multiple_catch_bodies)
{
    try
    {
        body();
    }
    catch (Exception e)
    {
        foreach (var @catch in multiple_catch_bodies)
            if (@catch.condition(e))
            {
                @catch.body(e);

                break;
            }
    }
}

This could be used as such:

@try(
    () => Console.WriteLine(1 / 0),
    e => Console.WriteLine("Uh-oh! Some Error occured: " + e.Message)
);

// catch with conditions (like 'when'-clauses)!
@try(
    () => Console.WriteLine(File.ReadAllText("/invalid/file_path")),
    new (Predicate<Exception>, Action<Exception>)[]
    {
        (e => e is ArgumentNullException, _ => Console.WriteLine("Argument was null!")),
        (e => e is FileNotFoundException, _ => Console.WriteLine("File not found!")),
        (e => e is IOException io_ex && io_ex.HResult == 42, e => Console.WriteLine("Something nasty: " + e)),
        (_ => true, e => Console.WriteLine("Some other exception: " + e))
    }
);

@stakx
Copy link
Contributor

stakx commented May 27, 2018

@Unknown6656 - Sorry, but in my opinion, your @try helper method is rather horrific. 🙈

  1. The resulting code looks bad.
  2. And it is hard to read.
  3. And it is going to perform worse than plain try..catch blocks. Your @try helper method involves a heap of avoidable allocations (a delegate instantiation for body, an array allocation for the multiple_catch_bodies parameters, and a delegate instantiation per item in multiple_catch_bodies). You're going to pay this allocation cost no matter what, even when no exception gets thrown.

@stakx
Copy link
Contributor

stakx commented May 27, 2018

That being said, I don't see why people are so bothered by regular try..catch blocks. Do they really make up such a significant part of an average code base that they deserve the attention? I agree that there are a few situations where it is necessary to have several nested try blocks... but in my experience that's fairly rare.

In my opinion, the => syntax in C# excels for short lambdas and short expressions (including throw expressions because they're usually short, too), but it is less ideal for statement blocks. With other words, => when used for catch blocks would only be appropriate if the catch block is reasonably short, and that's probably mostly the case with swallowing catch blocks. That's a practice that the language perhaps shouldn't encourage.

@alrz
Copy link
Member

alrz commented May 27, 2018

Should we label other proposals as Regular Proposal

@iam3yal
Copy link
Contributor

iam3yal commented May 27, 2018

@Unknown6656 Solves the problem but not as readable as a simple try/catch block. The only utility function like this that I can think of that would make sense and solves a very specific problem in this context and yet readable is what I suggested in this post although even then the downside of maintaining this in every project wouldn't worth the simplicity it brings.

@jnm2
Copy link
Contributor

jnm2 commented May 27, 2018

Should we label other proposals as Regular Proposal

Yeah, I think OP started off on the wrong foot here...

@dangi12012
Copy link
Author

All comments so far had in my opinion no compelling arguments. All the negative examples would also apply for get;set where the shorter syntax is objectively better. The only downside is that with short lambda try;catch nesting is not possible, but with the example I have provided the upside is obvious.

Also it would make get;set; and try;catch a homogeneus pattern which makes it easier to learn the language.

TL;DR: Shorter Syntax, More Readable, More Maintanable, Implementable by Syntactic Sugar

@jnm2
Copy link
Contributor

jnm2 commented May 27, 2018

@dangi12012 I'm trying to help. You're saying, "with the example I have provided the upside is obvious," so what everyone hears is, "You're all missing the obvious." Then you use a title-cased, bold tl;dr with a typo. How do you think this looks from someone else's point of view?

I love hearing a compelling argument. Neither of these tactics comes across to me as convincing, any more than leading off by demarking your proposal from everyone else's as a "good proposal."

@iam3yal
Copy link
Contributor

iam3yal commented May 27, 2018

All comments so far had in my opinion no compelling arguments.

I don't know what you're on about but the one that needs to provide a compelling argument is you.

I don't want to be harsh but adding [Good Proposal] to the title about exception handling when there are multiple proposals available and more importantly there are actually far better proposals that are more important to the language and to the .NET ecosystem than this one which makes it really hard to take this proposal seriously.

TL;DR: Shorter Syntax, More Readable, More Maintanable, Implementable by Syntactic Sugar

Less marketing statements; more facts.

Shorter? yes.

More readable? readability means easy to understand and easy to follow and the following code:

try {
    try {
        Foo();
    }
    catch(BarException) {
        return null;
    }
}
catch(Exception e) {
    Log(e.Message);
}
finally {
    Free(UnmanagedResource);
}

although verbose is in my opinion more readable than the proposed version.

More maintainable? that's just science fiction.

@dangi12012
Copy link
Author

dangi12012 commented May 27, 2018

Most common expressions have a short and canonical way of expressing a simple statement:

int Value //Shorter than old C# versions
{
     get => base.val;
     set => base.val = value;
}

if (true) Foo(); //No arrows needed because single statement works nevertheless
else Bar();

try => Foo(); //This needs to happen! 
catch(BarException e) => Log(e.Message);

Its so obvious that while the rest of the language evolved to a somewhat less verbose state,
Try;Catch has not evolved at all. (Allowed additional elegant syntax for canonical cases that is)

@Unknown6656
Copy link
Contributor

@stakx @eyalsk , OK, guys. You've got me ;)
That was one kind of ugly-ass code ......

To my defence, I wrote it only half-heartily and I am not convinced by my own static noise code.

I completely side with you guys on this issue.
Even though I think that try ... catch ... can be bulky when written in Allman style, I do not think that this proposal is a [Good Proposal]™.
It would be much better to look at the exception source in order to reduce the amount of trys in one's code.

@dangi12012 dangi12012 changed the title [Good Proposal] Expression-bodied try catch like get set [Good Proposal]™ Expression-bodied try catch like get set May 27, 2018
@Neme12
Copy link

Neme12 commented May 27, 2018

if (true) Foo(); //No arrows needed because single statement is special case!

No, it is not at all a special case. Look at the syntax for an if statement.
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/statements#the-if-statement
It's 'if' '(' boolean_expression ')' embedded_statement regardless of what embedded_statement is.

@Neme12
Copy link

Neme12 commented May 27, 2018

try => Foo(); //This needs to happen!

No it doesn't.

@jnm2
Copy link
Contributor

jnm2 commented May 27, 2018

dangi12012 changed the title from [Good Proposal] Expression-bodied try catch like get set to [Good Proposal]™ Expression-bodied try catch like get set an hour ago

I tried, but now I'm dying 🤣

@Neme12
Copy link

Neme12 commented May 27, 2018

It would be VERY cool to have this kind of lambda available for Try Catch:

Can you explain what this has to do with lambdas?

@Neme12
Copy link

Neme12 commented May 27, 2018

try
{
try => Foo();
catch(BarException) => return null;
}
catch(Exception e) => Log(e.Message);
Now try that with current state of C# and see how good it looks ;)

catch(BarException) { return null; }
catch(BarException) => return null;

You saved exactly 1 character. Forgive me if I don't see the added value.

BTW, return is not an expression so this doesn't really fit into your "expression bodied try catch" proposal.

@dangi12012
Copy link
Author

dangi12012 commented May 27, 2018

@Neme12 You clearly lack understanding of the language. Just look up the name of the => operator. Please dont spam this thread and try to come up with arguments. "No it doesn't." is not enough for a discussion.

Also the advantage is clear for this code:

try {
    try {
        Foo();
    }
    catch(BarException) {
        return null;
    }
}
catch(Exception e) {
    Log(e.Message);
}
finally {
    Free(UnmanagedResource);
}

Support for expression try/catch:

try
{
      try => Foo();
      catch(BarException) => return null;
}
catch(Exception e) => Log(e.Message);
finally => Free(UnmanagedResource);

Again:
Its so obvious that while the rest of the language evolved to a somewhat less verbose state,
Try;Catch has not evolved at all. (Allowed additional elegant syntax for canonical cases that is)

@Neme12
Copy link

Neme12 commented May 27, 2018

Please dont spam this thread and try to come up with arguments.

I'm not the one making a proposal and I don't have to make arguments to convince anyone. You do, and none of your arguments have been convincing so far. In fact none of them even make sense to me. Do you have anything other than "=> is VERY cool" and "let me save 1 character to avoid this boilerplate"?

@Neme12
Copy link

Neme12 commented May 27, 2018

Try;Catch has not evolved at all.

So? If statements haven't evolved either. Lots of things haven't "evolved".

@Neme12
Copy link

Neme12 commented May 27, 2018

Also the advantage is clear for this code:

Yes, if you add tons of whitespace to make the code longer. Whitespace doesn't count as boilerplate to me.

@Neme12
Copy link

Neme12 commented May 27, 2018

"No it doesn't." is not enough for a discussion.

"This needs to happen" is not enough either. Unless you provide a reason for why it does need to happen, it really doesn't.

@Neme12
Copy link

Neme12 commented May 27, 2018

Just look up the name of the => operator.

It is not called a "lambda".

@stakx
Copy link
Contributor

stakx commented May 27, 2018

I know I am not the police...

voodoo-stakx-transparent-small

... but this really isn't going anywhere productive, so for the sake of everyone's time and honour, may I humbly suggest that this issue be closed before it degenerates even further?

@Unknown6656
Copy link
Contributor

Unknown6656 commented May 28, 2018

Did someone say

before it degenerates even further?

Do I hear voices calling me? :trollface:

How about ...

Think about that one, LDT!

@DavidArno
Copy link

To my mind, this proposal is just trying to paint lipstick on a pig. It proposes slightly different syntax, but doesn't address the issue that throwing exceptions and using try ... catch as control flow is fundamentally not a very nice construct.

At some stage, C# will likely have discriminated union support, coupled with good pattern matching features. At that stage, try .. catch constructs can be replaced with stuff like:

bool TryFoo(Bar bar, out Foo foo)
{
    (var success, foo) = TryBar(bar) switch
    {
        Success<Bar>  s =>  (true, s.Value),
        BarError => (false, null),
        Error e => LogErrorAndFail(e)
    }
    return success;

    (bool, Foo) LogErrorAndFail(Error e)
    {
        Log(e.Message);
        return (false, null);
    }
}

No exceptions and program mechanics-exposing try ... catches, just simple, declaritive code that shows the intent and treats errors as a natural part of the program flow.

Until then, proposals for tarting up existing syntax seem a bit pointless to me. Forget evolution; we need revolution.

@asyncawaitmvvm
Copy link

Of course, there is value in being brief and concise - but that has to be weighed against other constraints as well. Having ideas like this one suggested over and over again doesn't make it them good ideas, just obvious ones.

I think you're treating your code as visual art rather than readable logic. There's a word for those braces: boilerplate. They are fine when you write Hello World but when you have 100 small functions that are each setting up an exception frame, it's a maintenance issue. Unfortunately #616 already figured it out. try/catch/finally should just following the bracing rules of other branching statements, where braces can be omitted with single lines.

@HaloFour
Copy link
Contributor

@asyncawaitmvvm

try/catch/finally should just following the bracing rules of other branching statements, where braces can be omitted with single lines.

Except that it can't because it's inherently ambiguous. Neither flavor of the syntax resolves that problem. The braces are a necessity to resolve that ambiguity.

@Neme12
Copy link

Neme12 commented May 29, 2018

It's not ambiguous if you disallow the embedded statement to be a try statement (the same should have been done for if IMO)

@DavidArno
Copy link

DavidArno commented May 29, 2018

@Neme12 ,

image

(Image "borrowed" from https://workbea.com/dont-put-lipstick-pig/)

@dangi12012
Copy link
Author

@HaloFour If it really was necesarry for braces to be there if, else would also disallow it.
If Else allows for some short syntax and so should try catch!

And for gods sake stop putting memes on this page.

@HaloFour
Copy link
Contributor

@dangi12012

If it really was necesarry for braces to be there if, else would also disallow it.

It probably should have. But an if statement can only have 0 or 1 else clauses, so a proscribed way to disambiguate is easier to reason about.

@orthoxerox
Copy link

While I agree that try/catch is rather verbose in its current state, I don't think adding expression-bodied forms to it is a worthy idea (in terms of added complexity). It will save some screen real estate, but will not improve exception handling as a process.

I'd rather have a try expression or catching branches in the switch expression or Result<T>/Validation<T> in the BCL with syntactic sugar to lift operations on them.

@dangi12012
Copy link
Author

dangi12012 commented May 29, 2018

Well no. Short IS better -> See get;set; if;else; Why not try;catch? There is no reason but its not implemented yet.

I guess you can always do with the new type matching: Where also lots of people said it is unnecessary - :sigh

try
{
    Foo();
}
catch(Exception e)
{
    switch(e)
    {
        case OperationCanceledException _:
        case InvalidProgramException _:
        case FieldAccessException _:
            Log(e);
            break;
        case OutOfMemoryException _:
            Environment.FailFast("Out of Memory");
            break;
        default:
            throw;
    }
}

@iam3yal
Copy link
Contributor

iam3yal commented May 29, 2018

@dangi12012

Short IS better

When there's a value to it; otherwise, not so much.

@iam3yal
Copy link
Contributor

iam3yal commented May 29, 2018

See get;set; if;else; Why not try;catch?

Why apples are not oranges? well...

@orthoxerox
Copy link

@dangi12012 the reason for not doing everything is the team at Microsoft having limited resources to implement everything. A lot of expression-bodied constructs were added to the compiler by a community member. Have you tried implementing expression-bodied try/catch in Roslyn? If you had a PR for that and could show that they don't break existing code and are perfectly legible you would receive a different treatment.

@chetankumar
Copy link

Well, almost all languages have some version of inline try catch.

Ruby especially is Brilliant with the "rescue" keyword.

I'm right now trying to delete files which may or may not exists. I don't want to wrap in a try catch..
File.Delete(path) rescue; (if it were ruby)

I would personally recommend a lambda expression like below:
File.Delete(path) catch(ex=> return ); // For void returning functions
int a = FindMinimum(int[] numbers) catch(ex=> return -1); // For methods which need to return a value.

This allows the option to do more with the exception, or not if not needed.

@TheFanatr
Copy link

TheFanatr commented Jul 8, 2019

Ok, so I know expression bodies are hot at the moment, but why overcomplicate things. IMO, try { } catch { } syntax could easily be converted to an expression via simply allowing the keywords to denote an expression similar to what was done with switch.

void CoolMethod() => DoAThing() catch (ChickenDiedException value) Console.WriteLine("Oh no!") catch (Exception e) Console.WriteLine(value.StackTrace);

If, however, we wanted to overcomplicate things, syntax could be allowed more modern-looking expression syntax.

void CoolMethod() => DoAThing() catch ChickenDiedException with Console.WriteLine("Oh no!") catch var value when ... with Console.WriteLine(value.StackTrace);

If we wanted to overcomplicate things to a further extent, then a similar body to the switch expression only with more limited pattern matching could be provided.

void CoolMethod() => DoAThing() catch
{
    var exception when ... => Console.WriteLIne(exception.StackTrace)
};

Instead of catch, this last pattern could be made an extensible part of the switch expression where a certain type could implement an operator to control the execution and choose a keyword to trigger the implementation with. In this case, the try or catch keywords could be used, so that instead of ... catch { ... => ... }, it would be ... try switch { ... => ... } or something similar so that this similar syntax is all unified as part of the switch expression. Note that I'm not sure how implementing such a system would work and is only a vague idea, where something better might look like a generalized expression syntax that can be customized, which this proposal's syntax and the switch expression would be a special case of.

@YairHalberstadt
Copy link
Contributor

Closing as duplicate of #220, #616, #786, #1068 and probably more

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