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

Proposal: try ... expressions #16072

Closed
gulshan opened this issue Dec 22, 2016 · 29 comments
Closed

Proposal: try ... expressions #16072

gulshan opened this issue Dec 22, 2016 · 29 comments

Comments

@gulshan
Copy link

gulshan commented Dec 22, 2016

Sometimes developers do not want to look at the detail type and detail information of an exception. They would just want to rethrow or return or carry on with a default value, when any exception has occurred. In those case, the try .. else ... expression can be handy:

var result = try ExceptionThrowingMethod() else throw; // To rethrow the exception
var result = try ExceptionThrowingMethod() else return; // To return from a void returning method
var result = try ExceptionThrowingMethod() else return 0; // To return from a method returning other types
var result = try ExceptionThrowingMethod() else new Object(); // To carry on with a default value
var result = try ExceptionThrowingMethod() else 0; // To carry on with 0 as a default value

And a try expression without else is just adding else null with the expression as default. That means-

var result = try ExceptionThrowingMethod();

is actually just,

var result = try ExceptionThrowingMethod() else null;

This feature may not be very good on its own. But along with proposed null checking in the C# 8, this can be handy. The try ... else ... syntax is actually borrowed from "The Error Model" blog from Joe Duffy, though was used in a fundamentally different(and more precise) error/exception handling model. This also depends on return expression proposed in #14239.

@iam3yal
Copy link

iam3yal commented Dec 22, 2016

@gulshan

And a try expression without else is just adding else null with the expression as default. That means

It shouldn't always return null though but the default value for whatever ExceptionThrowingMethod() returns, isn't? so I guess this is just an example.

@gulshan
Copy link
Author

gulshan commented Dec 22, 2016

The else part only comes into action when the try part throws. So, if ExceptionThrowingMethod() doesn't throw anything, var result will just receive the return value from the method, whatever it is.

@iam3yal
Copy link

iam3yal commented Dec 22, 2016

@gulshan Yeah, I figured.

What's the point of the following example?

var result = try ExceptionThrowingMethod() else throw;

Isn't this the same as doing this?

var result = ExceptionThrowingMethod();

I mean if you don't handle the exception and just rethrow, it seems pretty useless.

This would be equivalent to this: (ignore the assignment)

try
{
	ExceptionThrowingMethod();
}
catch
{
	throw;
}

But why would you do it?

@gulshan
Copy link
Author

gulshan commented Dec 22, 2016

Yes it's kind of pointless. Did not notice earlier. :(

@gulshan
Copy link
Author

gulshan commented Dec 22, 2016

Just came to my mind, may be explicit rethrows can be more useful (for logging etc) with proposed source generators-
https://github.com/dotnet/roslyn/blob/master/docs/features/generators.md

@alrz
Copy link
Contributor

alrz commented Dec 22, 2016

var result = try ExceptionThrowingMethod();

is actually just,

var result = try ExceptionThrowingMethod() else null;

So there is no need for else part as you can use null-coalescing operator, since try returns null if it throws.

var result = try ExceptionThrowingMethod() ?? defaultValue;

@YotaXP
Copy link

YotaXP commented Dec 22, 2016

So there is no need for else part as you can use null-coalescing operator, since try returns null if it throws.

Not necessarily. null could be a valid return value for the expression inside the try expression. For this reason, I also think I would rather the else clause be required. At least to start with.

@gulshan
Copy link
Author

gulshan commented Dec 22, 2016

My motivation here is, once the nullable reference types lands in C# and pattern matching becomes more powerful and mainstream, null will start to act more like the None as in the options in functional languages. Nullable reference and value types will be like the option wrapper over the non-nullable types. So, a failure/error/exception will become synonymous to null. This try expression will support that notion by converting raised exceptions to nulls, without trying to read the exception.

@alrz if the explicit rethrow is discarded (which is not really adding much value as pointed by @eyalsk ), your proposal with null-coalescing ?? operator makes more sense to me. Because then it will be using existing language constructs and thus will require less changes in grammar.

But @YotaXP 's question should also be taken into account. Is there any situation, where a method returns null as a normal output and not to denote some kind of error? If exists, how much common are those situations/practices. Some examples perhaps?

@svick
Copy link
Contributor

svick commented Dec 22, 2016

I believe that the commonly accepted good practice is that general catches should be avoided (it's even an FxCop warning). Instead, under normal circumstances, you should only catch exceptions that you know how to handle.

There are situations where those suggestions don't apply, but I don't think it's a good idea to add syntax to the language, which would make code shorter in those rare situations, but that would also lead to people who should follow the common good practices to ignore them (because it's shorter). C# should be a pit of success, not pit of despair.

@jnm2
Copy link
Contributor

jnm2 commented Dec 22, 2016

I believe that the commonly accepted good practice is that general catches should be avoided (it's even an FxCop warning). Instead, under normal circumstances, you should only catch exceptions that you know how to handle.

There are situations where those suggestions don't apply, but I don't think it's a good idea to add syntax to the language, which would make code shorter in those rare situations, but that would also lead to people who should follow the common good practices to ignore them (because it's shorter). C# should be a pit of success, not pit of despair.

This.

@iam3yal
Copy link

iam3yal commented Dec 23, 2016

@gulshan

But @YotaXP 's question should also be taken into account. Is there any situation, where a method returns null as a normal output and not to denote some kind of error? If exists, how much common are those situations/practices. Some examples perhaps?

null is not really an indication to an error, you generally return null because an operation ended and you can't really return a proper value.

Just as an example you can think about a DB operation that returns null when the operation succeeded but no records were found, some people might choose to return an instance of some class that represents this case like in the null object pattern but others will happily return null.

There's at least three problems with the null object pattern:

  1. It isn't transparent so you wouldn't know whether a function returns such an object so it must be a convention so everyone on the project and customers alike need to know that functions never return null but an object so there's no need to check for it, otherwise, you shoot yourself in the foot.

  2. It's not always possible to derive from the class to create a null representation of this object.

  3. There's some allocations associated with it.

@Unknown6656
Copy link

I would rather like to see something like this allowed:

try
    method();
catch (Exception ex)
    errorhandler(ex);

(Allowing the removal of } and { on single-expressions inside try/catch/finally)

@jnm2
Copy link
Contributor

jnm2 commented Dec 23, 2016

(Allowing the removal of } and { on single-expressions inside try/catch/finally)

Yes! Me too.

@gulshan gulshan changed the title Proposal: try ... and try ... else ... expressions Proposal: try ... expressions Dec 24, 2016
@iam3yal
Copy link

iam3yal commented Dec 24, 2016

@Unknown6656, @jnm2 Make a proposal about it unless it exists. :)

@Thaina
Copy link

Thaina commented Dec 24, 2016

Would like it to be dorpped try and ; too

method() catch (Exception ex) errorhandler(ex);

Then

var value = method() catch null;

Maybe lambda?

var value = method() catch((Exception ex) => {
    return null;
});

var value = method() catch((Exception ex) => null);

@iam3yal
Copy link

iam3yal commented Dec 24, 2016

@Thaina Removing arbitrary stuff doesn't make things clear but the opposite, it looks both ugly and broken.

p.s. You guys are proposing things that aren't really relate to this proposal so in my opinion you should create a new issue about it.

@iam3yal
Copy link

iam3yal commented Dec 24, 2016

@Thaina Well, okay, can you please make a new issue about it? :)

@Thaina
Copy link

Thaina commented Dec 24, 2016

@eyalsk Well, I don't think about make it more clear. Just want to make it short and compact. One line cast if possible

I want to make it shorter from

int x;
try { x = SomeMethod(); }
catch(Exception ex) { x = 0 }

Want something like

var x = SomeMethod() catch 0;

@jnm2
Copy link
Contributor

jnm2 commented Dec 24, 2016

For the last time, catch-all is harmful and should not be encouraged. 🙄

@iam3yal
Copy link

iam3yal commented Dec 24, 2016

@Thaina Generally when people come up with a syntactic sugar it's because there's a pattern that is usually a good practice but at the same time it may create many repetitions across the code so sometimes baking it into the language might make sense but as people have noted here swallowing exceptions isn't a good practice hence why the suggestion is bad, now on top of that you're asking to have an alien syntax that makes it twice as bad.

@Thaina
Copy link

Thaina commented Dec 24, 2016

@eyalsk Yeah I just make up that syntax on a whim. Don't think it look good just make it work as I expected

But my idea actually came from a pattern. The TryGet function pattern

You could see there are TryGetValue from dictionary or TryParse from most number struct. Normally Parse function with invalid string or Get what not contains will throw exception. But with Try it would give you default value without exception. And that's what needed in the most case

Or if you just concern that we should need try keyword in the front?

I just think it not really necessary. We already have catch

@jnm2
Copy link
Contributor

jnm2 commented Dec 24, 2016

Well, the difference is that TryGetValue and TryParse do not catch exceptions; using them, exceptions are never thrown in the first place. That's the real win.

Exceptions are for exceptional situations. Parse is for when you are absolutely sure it won't fail (or that if it does, the responsibility is higher up the call stack). TryParse is for when the input might not be parseable, and therefore failure to parse is not an exceptional circumstance.

@Thaina
Copy link

Thaina commented Dec 24, 2016

@jnm2 I mean int.Parse is a function that throw exception

So you need to make int.TryParse to make it not throw exception

So I was argue that, this is a pattern and we should just have syntactic sugar for it

@jnm2
Copy link
Contributor

jnm2 commented Dec 24, 2016

@Thaina

Yes, I understand what you're saying. The problem is you can't generalize that pattern. TryParse does not call Parse and it does not catch exceptions. TryParse has its own special implementation and that is necessary. If we had such syntactic sugar, try int.Parse would call Parse and it would catch all exceptions and I don't think that's a good thing.

You should never catch exceptions except very specific ones where you're going to deal with it in a domain-specific way and your caller should not be allowed to be responsible for it. This applies to syntactic sugar as well as normal catch blocks. This does not apply to TryParse where no exceptions are thrown.

@iam3yal
Copy link

iam3yal commented Dec 24, 2016

@Thaina

The try/catch comes with some performance penalties so the BCL team came up with the Tester-Doer pattern to combat this for methods that may throw on failure and throwing and catching them is expensive, more so when you need to do that many times so they introduce a Try version for these methods and these don't throw at all, in fact their internal implementation is different.

You could see there are TryGetValue from dictionary or TryParse from most number struct. Normally Parse function with invalid string or Get what not contains will throw exception. But with Try it would give you default value without exception. And that's what needed in the most case

It's very different from what you're proposing, if you will compare for example Int.Parse and Int.TryParse you will notice that as I stated above that their implementations is different so in this case they aren't just swallowing exceptions but providing different implementation to avoid exception handling completely.

It's true that in some cases where you don't have such methods you can either swallow the exception or provide an alternative implementation, in fact, in some cases swallowing can be faster but 80% it isn't, meaning, to find out you really need to do some benchmarks.

So I was argue that, this is a pattern and we should just have syntactic sugar for it

Again, it seems like you're thinking that Int.TryParse wraps Int.Parse and swallow the exception but it's not really the case.

@Thaina
Copy link

Thaina commented Dec 24, 2016

@jnm2 I think opposite, we can generalize it. And it work like you just say. try it, catch exception, then set the default value

That's what we always did for function that we don't know would it cause exception but we just really need to have a value set. Such as 404 or 500 from HttpWebRequest

@eyalsk Well I know you are right about performance and I too hate to do try/catch because that reason too. And if I could write any function that could receive invalid argument by myself I would always make TryXXX function

But we cannot control what 3rd party would implement. They would throw everything they don't like without concern about performance because they write function to work in server. They could just throw 404 or 500 HttpException

And we need to deal with it anyway, I cannot avoid try/catch when work with 3rd party. So I just want syntactic sugar for it

I know int.Parse and int.TryParse implement differently internally. But that's not need to be anyone concern. There are API and we just use it

int.TryParse might or might not really wrap int.Parse but for the API consumer, it is as what I say. int.Parse is throw exception version. And int.TryParse is not throwing version. That's all we need to know

@gulshan
Copy link
Author

gulshan commented Dec 25, 2016

Thank you all for the discussion. It is driving me to think of the alternatives and refining my idea of how the try expression should look like. Now I think, instead of swallowing the exception and returning null, the try expression should return a Result object. The definition of object using the proposal #6739 will be-

public enum class Result{
    Success<T>(T Value),
    Error(Exception Ex);
}

The try expression will simply then catch and return the exception wrapped in a Error object, if a exception is thrown. Otherwise, it will return the actual return value, wrapped in a Success<T> object. So the exception handling will become-
If implemented, this means the else part will come for free. The error handling will look like-

var result = try ExceptionThrowingMethod();
if(result is Error e){
    //handle error here and return/throw it, or anything
    //else to get out of enclosing scope
}
var resultValue = (result as Success).value;
System.Console.Writeline(resultValue);
// use resultValue for further work

And I have found this line describing destructuring in proposed patterns document-

| 'let' pattern '=' expression 'else' embedded_statement

If implemented, this means something like the else part of the original proposal will come for free. Then error handling will look like-

var result = try ExceptionThrowingMethod();
let Success goodResult = result else {
    System.Console.Writeline((result as Error).Ex);
    //handle error here and return/throw it, or anything
    //else to get out of enclosing scope
}
System.Console.Writeline(goodResult.value);
// use goodResult.value for further work

And if someone wants to go the discouraged exception swallowing route-

let Success resultValue = try ExceptionThrowingMethod() else return;

I think there may be some problem with matching a generic type Success<T>. There has to be another issue discussing that to make C# infer more types.

@gulshan
Copy link
Author

gulshan commented Mar 4, 2017

I want to wait for rolling out of ADT in C# and further discussion should happen on csharplang repo.

@DavesApps
Copy link

@gulshan a variation on this idea here:
dotnet/csharplang#965

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests