-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Yet another try expression proposal #980
Comments
@gulshan The following code:
is in my opinion more readable than the following:
I don't understand all these exception handling proposals, especially, the ones that propose to make it easier to swallow exceptions, what's the real benefit here? besides make the syntax terser and "cuter" which conveys the opposite of what many experienced C# devs would consider an anti-pattern.
The anti-pattern is about swallowing exceptions not about what you swallow so I'm not sure why swallowing ApplicationException, SystemException would be fine whereas swallowing Exception wouldn't. In some circumstances it's fine to swallow exceptions and I don't think that educating developers by preventing them what's available through another syntax is the way to go, especially when this is a syntactic sugar and nothing more. Finally, if we're going to have a terser syntax I'd rather have what @CyrusNajmabadi suggested and incorporate it into
For anything more complex, you can always fall back to the "awesome" syntax we have today. Again, just my opinion about it. |
I get wanting terser syntax. But I'd prefer to have language updates that support functional error handling, similar to F#'s |
@eyalsk I would not create this issue unless the discussion happened in this thread- https://github.com/dotnet/corefx/issues/19928 @bondsbw C# is so much so into the try-catch style error-handling, it will be very difficult to introduce another way of doing IMHO. |
@bondsbw you can use a library for that. I know language-ext supports that feature out of the box, not sure about SuccincT. |
Fair enough, the issue though is the proposal on CoreFX is an API and there's already existing code that using the Tester-Doer Pattern all over the framework, do you expect them to change these APIs and mark them obsolete or do you expect consumer to change their code? in my opinion you should address that in your proposal too.
It wasn't a proposal, it was comment. :D
It is and I already provided at least two more, ApplicationException, SystemException and these probably aren't the only ones. Now, I've seen codebases where people swallowing exceptions for the wrong reasons plenty of times and a developer once told me that he hates handling exceptions so he swallows them so I offered different options but the answers I got were completely detached from reality such as "It's too much code where I can just swallow it" or "it's just easier to swallow it than deal with it" even when I pointed out that it's even shorter not to swallow it and then you get answers like "yes but the application will crash" so you stop there and realize that the developer is well, unfortunately, clueless and so when reason doesn't matter to some developers I wouldn't want the language to make this any easier and at the same time I wouldn't want the language to educate me about what I swallow when I need it and there's a good reason to do it. This being said swallow Just a silly example but what if someone relies on some 3rd-party library where this library throw instances of |
Good points. We definitely need code quality analyzers though to warn on |
Warning Waves? 😄 |
Overall, I want something like this. I'd prefer that you have to be explicit about returning a default value, but this can be pretty nice because we can allow null without an explicit cast just like the proposed Adding var x = try GetInt() catch FooException: null, catch BarException ex: ex.Number;
var x = try match GetObj()
case int n: n,
default: 0,
catch FooException: null,
catch BarException ex: ex.Number; Also what about BeginX();
return try DoOperation() finally EndX(); Meh? |
Of course, the problem with composing var x = match try GetObj()
catch FooException: null,
catch BarException ex: ex.Number,
case int n: n,
default: 0; Which is starting to feel awkward and yet is quite expressive in its way of what's going on. Or different whitespace: var x = match
try GetObj()
catch FooException: null,
catch BarException ex: ex.Number,
case int n: n,
default: 0; |
Composing either way also has the issue that either the catches are applied to the cases or the cases are applied to the catches. You can't have everything apply only to the expression unless you include I guess I do find that language mechanic the most appealing right now. |
@jnm2 Using var result = try SomeOperation()
catch FooException
catch (BarException e) when (e.ErrorCode == 0x1234)
?? 0; As it was a totally new syntax, I took the liberty of making blanket catch-alls ( I don't see |
An expression version of |
Actually using var result = try SomeOperation()
catch FooException ?? 0
catch BarException ?? 1
catch BazException ?? 2;
// This should be evaluated like-
var result = (
(
(try SomeOperation()
catch FooException) ?? 0
catch BarException) ?? 1 // Exception occurred, escaped FooException catch
catch BazException) ?? 2; // Exception occurred, escaped previous catches With this even fall-through will be ok- var result = try SomeOperation()
catch FooException
catch BarException ?? 0 // value for both FooException and BarException
catch BazException ?? -1; @HaloFour I've probably seen the |
I don't like this because if the |
Any syntax such as this will be inherently much slower than specialized |
Yes! Please do not confuse the |
@jnm2 I think that depends on the implementation. I don't think exception thrown by an expression in place of |
@HaloFour the proposal #965 offers a suggestion that the implementation could be instead of throwing exceptions when a function is wrapped by trycatch it would be recompiled to RETURN the exception rather than throwing it, to avoid the performance overhead of the exception. So basically it does rewrite a method to some extent. It won't be as good as a custom one but in theory during recompile of a method it could disable finalization options for the exception which should significantly improve the performance of the create and clean up. |
You're suggesting that by wrapping an arbitrary method that it will be decompiled, reinterpretted and rewritten to not throw? And this will happen with library methods? How do you propose that work? The compiler certainly has no capacity to affect the nature or code of referenced assemblies which means that you're expecting the JIT to somehow completely rewrite methods at runtime. I can't see that remotely getting off of the ground. |
And to expand on that further, many of those |
@HaloFour when it's jitted it could just replace return values with the tuple and replace throw with return the tuple. Jit happens on all methods at runtime. It should be a relatively simple replacement/addition of the IL to Jit during generation. (It was just a suggestion at solving this ubiquitous problem that lots of people are asking for all different sorts of methods with Try on it to avoid exceptions... Any other ideas that could solve having to write regular methods and Try methods all the time?) Seems like enough folks run into this once in a while would be nice if we could find a general solution. |
Given that literally any method can throw this is asking the JIT to be on standby for rewriting literally every single piece of IL that passes its way. That includes constructors which cannot modify their return values. I don't think that's practical. It's also not a proper replacement for |
Obviously, we need a try/catch expression. The above proposal failed to handle the ambiguity of multiple nested try/catches - in such a case it's impossible to tell which catch corresponds to which try. Instead, I'm suggesting a syntax that should be pretty idiomatic to C# 8.0 and later. C# 3.0 introduced lambda syntax, C# 7 introduced pattern-matching, and C# 8 introduced switch expressions. Fundametnally a Catch is a pattern-matching operation on the Exception object, so we should reuse the pattern-matching-expression syntax here. proposal By toy example first:
as you can see, slightly noisier than a normal Try-Catch, but still terse and clear, and leverages familiar Lambda, Switch expression, and pattern-matching syntaxes. More elaborate example.
so you can see, the pattern is
where all of If an exception expression needs to be broken out into a multi-statement body, then the syntax already established for doing the same for lambdas is used.
This also avoids the ambiguity problems - the lambda-arrows make it unambiguous compared to a normal try...catch (an existing try will never be followed by a lambda arrow). Because all the catch blocks are in a single Using braces to contain multiple No existing try...catch features such as multiple catch-blocks, when-clauses, etc. are sacrificed other than "finally", and the empty |
@Pxtl That aligns pretty well with I'm not sure I like it more than adding it to |
@bondsbw yeah, I'm not sure the first What do you mean by "than adding it to switch expression syntax"? After all, that ship has sailed - switch expression syntax, including arrows, is in C# 8. |
@Pxtl Why do you need |
I don't think it would be a breaking change to add |
@bondsbw I'm still not following - add
? |
@Pxtl more like add var x = doSomethingThatCouldFail() switch {
catch ex => logAndReturnNull(ex),
case var _x => _x,
}; |
@orthoxerox Switch expression does not use |
@Pxtl It was covered above in the thread. It needs to be updated slightly for the decided |
After finalization of the switch expression, I don't support my proposal in this issue now and want to close it, when discussion here is wound down. I think the proposal by @Pxtl is a good one because of its syntactic alignment with the switch expression and deserves its own proposal/discussion issue. Some minor points from my side regarding it-
With these the example code given by @Pxtl will look like- var responseCode = try doStuff().ReponseCode catch
{
NotFoundException => 404, //don't need ex variable, use immediate value.
HttpException ex when (ex.Code < 500) => ex.Code, //just pass-through the response-code
HttpException ex => logger.LogErrorAndReturn(ex, e => e.Code), //for 500 and up errors, log the error before returning
Exception ex => logger.LogErrorAndReturn(ex, _ => 500), //treat all other errors as 500s.
}; Regarding putting catches in the switch expression, I think putting a var value = try int.Parse(stringValue) switch
{
int x => x,
ArgumentException e => 0,
FormatException e => 0,
} |
In my current project, I've got an In that, I have a static method: public static Outcome<T> Try(Func<Outcome<T>> tryFunc, Func<Exception, Outcome<T>> catchFunc)
{
try
{
return tryFunc.Invoke();
}
catch (Exception ex)
{
return catchFunc(ex);
}
} Which is then used as: Outcome<int>.Try( () => DoSomething(), ex => Outcome<int>.Error() ); It's a little more syntax heavy that just |
I'd agree with that. So @gulshan , I'm not familiar with the chsarplang process here - do we create a new proposal issue and close this one? |
@Pxtl Just create a new issue with your proposal, like you did in the roslyn repo. I'll close this issue later. |
It seems recently try-... expression issue came into discussion again. I have some new ideas regarding this. So, this "yet another" try expression proposal. Please bear with me.
This is actually a two(or three) layered proposal, where each layer is built on the previous one.
First layer: Support catch blocks with try expression if it returns or throws.
The idea is, with try expression, support catch blocks which eventually ends the current method with
return
orthrow
. It will look something like-And same goes for throws. The first layer ends here.
Second layer: Support non-returning and non-throwing catch blocks with certain regulations.
Supporting non-ending catch blocks comes with two problems. First is, what will be the value of the expression in the case of exception. Second is, wouldn't this enable exception-swallowing anti patterns?
I think there can be three values an expression will evaluated to in case of exceptions-
null
default
I tend to like the
null
option most as it denotes absence of a value and also enables??
operator if someone likes to put a default value for fail cases.Now let's address the big question. This will enable to swallow exceptions, which is an anti-pattern. I think, simply swallowing an specific type of exception is not as bad as swallowing ALL exception. So, lets make (non-method ending)all-swallowing catch blocks like
catch {...}
orcatch (Exception e) {...}
illegal in try expression. One cannot simply swallow all. The code will show exactly what it is swallowing, which is better and enough IMHO. Is this restriction enough for you?
Third layer: Simplify catches
If all I want to do is to catch and swallow some specific types of exception without doing anything else and get
null
s, why not make it easier? Make the parens and "block" part of a catch clause optional and let multiple types of exceptions be combined with|
or,
. This will enable to code like-So, a sample try-catch expression demonstrating different parts of this proposal is-
Parent issues: dotnet/roslyn#16072 #220
Recent discussion: https://github.com/dotnet/corefx/issues/19928
cc: @jnm2 @HaloFour @KrzysztofCwalina @jaredpar
The text was updated successfully, but these errors were encountered: