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

Discussion: try expression without catch for inline use #220

Closed
lachbaer opened this issue Mar 3, 2017 · 124 comments
Closed

Discussion: try expression without catch for inline use #220

lachbaer opened this issue Mar 3, 2017 · 124 comments

Comments

@lachbaer
Copy link
Contributor

lachbaer commented Mar 3, 2017

Proposal: try expression without catch for inline use

Intent

Sometimes it is not necessary to catch an exception, because you can either check side conditions or proceed though the expression is failed. In these cases it would be nice to skip the exception checking.

Example

In current code there would be something like

try {
    var textStream = new StreamReader("C:\nonexistingfile.txt");
    ProcessFile(textStream);
}
catch { }
GoOnWithOtherThings();

or even wrap it additionally with

var fileName = @"C:\notexisting.txt";
if (File.Exists(fileName)) {
   // try block from above
}
GoOnWithOtherThings();

This could be abbrevated and streamlined drastically with

var textStream = try new StreamReader("C:\nonexistingfile.txt");
if (textStream != null) ProcessFile(textStream);
GoOnWithOtherThings();

The catching isn't necessary here, because a null-check (that should be done in ProcessFile() anyway) already cares about the failure. A complete try { } catch { } is just unnecessary boilerplate.

I guess that there are plenty of other useful scenarios where this would come in handy.

Other languages

PHP uses the @ operator before an expression to suppress errors and warnings

$fp = @fsockopen("www.example.com", 80, $errno, $errstr, 30);
if (!$fp) {
    echo "$errstr ($errno)<br />\n";

In PHP @ is used quite often to shorten things.

Now, open for discussion... 😄

@orthoxerox
Copy link

I'm sure this change will push people not into the pit of success, but into the cliffs of insanity. There's a reason why "On Error Resume Next" is the most offensive thing you can say to a VB programmer.

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

@orthoxerox
In VB On Error Resume Next has been the only way to at least simulate something like a try ... catch, because the code would not have thrown a "Run time error" and one could examile the Err.Number by hand.

Nevertheless I understand your concerns, but I don't think that it will have too much impact. As with so many company conventions, there could be one that disallows usage of a try expression when not thorougly documented why it is used.

For all others this would much probably be an ease!

@DavidArno
Copy link

In PHP @ is used quite often to shorten things.

PHP, the good parts

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

And by the way, actually it wouldn't be much different than using TryParse() - a framework supported feature.
Other question: haven't you too already seen used code with empty catch blocks anywhere? If not, your bad 😉

@DavidArno
Copy link

public static StreamReader TryNewStreamReader(string filePath)
{
    try 
    {
        return new StreamReader("C:\nonexistingfile.txt");
    }
    catch { }

    return null;
}
...
var textStream = TryNewStreamReader("C:\nonexistingfile.txt");
if (textStream != null) ProcessFile(textStream);
GoOnWithOtherThings();

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

@DavidArno are you kidding me....?!?!?!?! 😕

var textStream = TryNewStreamReader("C:\nonexistingfile.txt");
// compare to
// var textSTream = try new StreamReader("C:\nonexistingfile.txt");
if (textStream != null) ProcessFile(textStream);
GoOnWithOtherThings();

The complete "TryNewStreamReader()" is so something of spoiling overhead! And besides you used an empty catch block in it. Exactly what this proposal is going to avoid!
;confused; ;confused; ;confused; 😕

@DavidArno
Copy link

How do you propose avoiding the empty catch block? I simply copied your example code.

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

@DavidArno

How do you propose avoiding the empty catch block? I simply copied your example code.

That's the whole point of this proposal.

var textStream = try new StreamReader("C:\nonexistingfile.txt");

is all that is needed. It is semantically equivalent to

try {
    var textStream = new StreamReader("C:\nonexistingfile.txt");
 } 
catch { }

but without the boilerplate around it.

@DavidArno
Copy link

catch { } is evil. There are times when it might make sense to use it after careful analysis of the consequences. Having the compiler encourage its casual use through a syntactic shortcut though, would be a seriously bad thing to do.

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

@DavidArno I like to look at things without any prejudice or preassumptions and from different angles from time to time.

During my early days I have teached beginner programming classes. That was around the time of C# 1.0 (the classes were for Pascal though). That again teached me to "dive into the thread of thoughts" of others, to see where their problem of understanding is or why they wrote the code as it was when it came to debugging.

From a theoretical and operational point of view, I totally agree, that every exception should be caught and thought of wisely. From a practical and more humal like POV in my eyes it isn't that bad to omit exceptions.

It's due to the vast number of exceptions that could throw anytime. Non-experienced programmers could easily be discouraged, because it takes too much time to respect them here and there. And all this try and catch clauses with all the braces and indenting makes the code harder to follow, also working against people who already have a hard time to concentrate of the essential algorithms.

When you - without prejudice! - look at some code ( I don't mean enterprise class code, just everyday laying around code ) you will most certainly find quite a lot of catching that actually isn't really necessary, or only exists to catch the annoying occurences of exceptions. In my eyes a simple try expression as proposed here would extremely enhance that already existing code.

By the way, in this forum are mainly enthusiasts who might have a more professional look on things. Please respect that there are others on this planet who love C# and program for fun, but find this or that annoying and improvable. (And professionals will probably ignore features like this proposed one anyway - and if not, then there might be a use for it, indeed 😉 )

@FredyFerrari
Copy link

I just don't think that a language should help programmers to write bad code but guide them to write proper code. And, in my opinion, a catch all clause is always dangerous and therefore should not be something that the compiler should insert automatically.

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

@FredyFerrari
Please look it at from a diffrent angle! You could also see it this way:

By now the language encourages to write catch all clauses, that also look ugly, leads to additional braced blocks and indention:

try {
    var textStream = new StreamReader("C:\nonexistingfile.txt");
 } 
catch { }

When do I write that kind of code? => When I'm too lazy to catch the exception right now, because I concentrate on the actual work of the module. Maybe I come back later to care about exceptions. ( 'I' is not ment personally )

So all I have done is to insert that try / catch intentionally - already.

Inlining that very try would not have me encouraged to write bad code - I would have written that bad catch-all-code anyway. It now just looks tighter and cleaner with respect to the reading flow.

The IDE can always transform it to a full try-catch-block (e.g. one-way only) and help me finding inlined try statements. Even better! Inlined try statements are syntactically easier to find for code enhancers and -reporters and thus can hint you where you should further inspect your exception handling, isn't it?

@FredyFerrari
Copy link

But allowing this would make people think, var textStream = try new StreamReader("C:\nonexistingfile.txt");
is a proper way to write code and not make them aware that this catches all exceptions.

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

@FredyFerrari
But do you admit that

var textStream = try new StreamReader("C:\nonexistingfile.txt");

looks better than

var textStream = try { new StreamReader("C:\nonexistingfile.txt"); } catch { }

? I myself do not like the extensive use of curly braces, e.g. like here (besides it wouldn't compile).

@iam3yal
Copy link
Contributor

iam3yal commented Mar 3, 2017

@lachbaer

And by the way, actually it wouldn't be much different than using TryParse() - a framework supported feature.

Point of the Tester-Doer pattern is to avoid the exception not to swallow it, so it's very different.

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

Well, how about this:

    var textStream = (
        (Func<StreamReader>)
        delegate () {
            try { return new StreamReader("C:\nonexistingfile.txt"); } catch { return null; }
        })();

vs.

var textStream = try new StreamReader("C:\nonexistingfile.txt");

😂 😂 😂 😂

@DavidArno
Copy link

@lachbaer,

By now the language encourages to write catch all clauses

You are right, and it's a historic flaw in the language. It's easier to write a bad catch-all than it is to eg write:

try
{
    var x = new StreamReader("C:\nonexistingfile.txt");
    ...
}
catch (Exception e) when (e is ArgumentException ||
                          e is FileNotFoundException ||
                          e is DirectoryNotFoundException ||
                          e is IOException) {}

There's nothing we can do about that historic problem. However, making it even easier to write catch-all's, as per this proposal, is just digging that "pit of failure" hole even deeper.

@iam3yal
Copy link
Contributor

iam3yal commented Mar 3, 2017

@lachbaer There! you even get the PHP syntax you always wanted.

public static class Utilities
{
    public static T @try<T>(Func<T> construct) where T : class
	{
		try
		{
			return construct();
		}
		catch
		{
			return default(T);
		}
	}
}

All you need to do is put using static Utilities at the top. 😆

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

Exception handling can be more performant than test-doing
https://www.dotnetperls.com/tester-doer

And image the code on that page with the proposed try statement...

@gulshan
Copy link

gulshan commented Mar 3, 2017

I would rather support a try-catch expression, which returns a value if no exception, but on exception executes the catch block and do not proceed after that. In void returning methods, it just returns after executing catch block. For other methods, it's an error not returning within the catch block. Similar to proposed guard syntax. For example-

try {
    var textStream = new StreamReader("C:\nonexistingfile.txt");
    ProcessFile(textStream);
}
catch (Exception e) {  Logger.Log(e); }
GoOnWithOtherThings();

becomes

var textStream = try new StreamReader("C:\nonexistingfile.txt")
                   catch (Exception e) { Logger.Log(e); }
// textStream is definitely defined at this point
// and no exception thrown in the try expression evaluation
ProcessFile(textStream);

// In case of exceptions, this method will not be executed.
GoOnWithOtherThings();

But if you want to swallow, you have to do it explicitly-

var textStream = try new StreamReader("C:\nonexistingfile.txt") catch { return null; }
ProcessFile(textStream);

@iam3yal
Copy link
Contributor

iam3yal commented Mar 3, 2017

@gulshan You should make a new proposal about it as it is divergent.

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

However, making it even easier to write catch-all's, as per this proposal, is just digging that "pit of failure" hole even deeper.

I completely disagree! As you say, it is already there. I think I get you right if you would have it closed completely, but that will most likely never be the case.

Please think again about what I wrote concerning the help of code analyzers. When people start using a try statement instead of a catch-all block these occurences could be easily highlighted and complained by an analyzer. They will write those catch-all's anyway! So, in this view it even encourages to inspect those places and to refactor to meaningful catches.

It also enhances writability, because by using the try statement you can concentrate on the main issue, but also marking places that you gonna have to inspect later.

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

var textStream = try new StreamReader("C:\nonexistingfile.txt") catch { return null; }

Actually, I think the main purpose for my suggestion of a try-statement is for assignments. So, I could go with his. However, catch { return null; } again is too much boilerplate to me.

As this is for returning it could be

var textStream = try new StreamReader("C:\nonexistingfile.txt")
                 catch => null;

@gulshan
If you make a new proposal, I will close this one in favor of yours.

-- Addendum --
I'd rather have the try being an expression body, because that implies a possible return value. The catch shouldn't return anything.

var textStream = try => new StreamReader("C:\nonexistingfile.txt");
                 catch { /* ordinary catch block */ };

Or if you prefer a no-catch one-liner

var textStream = try => new StreamReader("C:\nonexistingfile.txt"); catch {}

would translate to

StreamReader textStream = null;
try {
    textStream = new StreamReader("C:\nonexistingfile.txt");
}
catch { /* ordinary catch block */ };

In case you just need catch to return anything else:

var textStream = ( try => new StreamReader("C:\nonexistingfile.txt"); catch { } )
                       ?? new StreamReader("C:\existingfile.txt");

@DavidArno
Copy link

DavidArno commented Mar 3, 2017

@lachbaer,

Since @eyalsk has provided a simple function that you can include in any of your code, that offers almost exactly the syntax you want:

var textStream = @try(() => new StreamReader("C:\nonexistingfile.txt"));

I don't really see the point of @gulshan creating a new proposal.

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

@DavidArno
The point is to make that common pattern available in the language for everybody, not only for those who know the function. I thought that's obvious. 🙁

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017

@DavidArno Besides a "try-catch-expression" goes far beyond that one written function.

@jnm2
Copy link
Contributor

jnm2 commented Mar 3, 2017

-1. I have witnessed first-hand the horrors of catch-all blocks. Unless you are a message loop or threading framework, you have no business swallowing exceptions that don't belong to you. (And if you are, you re-throw them.) It's both dangerous- by far the safest action for any business is for the program to immediately crash on an unexpected exception rather than run in an unknown state- and frustrating because other code swallowing makes it impossible for you to handle the exception higher up the call stack, even if the exception is your responsibility to know about and handle!

When this happens in real life, you gain more perspective than simply the fallacy that it's okay to swallow all exceptions.

@HaloFour
Copy link
Contributor

HaloFour commented Mar 3, 2017

On Error Resume Next

@lachbaer
Copy link
Contributor Author

lachbaer commented Mar 3, 2017 via email

@HaloFour
Copy link
Contributor

HaloFour commented Mar 3, 2017

catch { } should be the very antithesis of boilerplate code.

@lachbaer
Copy link
Contributor Author

@CyrusNajmabadi
In any case catch is allowed in match expressions, the try keyword should also be used!

var v = try DoSomething() match (
    case SomeType t: Something(),
    case OtherType x: Whatever(),
    catch (Exception e): AndSoOn());

// or (not my choice)
try var v = DoSomething() match ( ... );

So, a try forces at least one catch-case to be defined in the match expression and vice versa. And that catch expression must have a parameter!

@iam3yal
Copy link
Contributor

iam3yal commented Mar 10, 2017

@Thaina

And your suggestion on return is ambiguous that will it return to variable or out of function.

Not really, the syntax is invalid and can be compiled exactly to what you proposed in your post so it isn't ambiguous.

@lachbaer
Copy link
Contributor Author

Having used try-catch lately, I'm going to update my opinion.

  1. try should stay with a block body (or expression body, but that's probably not gonna come...)
  2. try expressions would really be useful.
    I think the best approach is with the with expression as stated above.
  3. catch clauses shall be allowed to be block-less!!!
    There are so many cases where the work of a catch can be accomplished in only one line.
    To circumvent the problem with nested trys, another try in a body-less catch handler must not be allowed.

These modifications would already pump up coding 😄

@lachbaer
Copy link
Contributor Author

lachbaer commented May 8, 2017

Two different ideas to this proposal can be seen here:

var x = try CalculateInt() catch(Exception e) => 0;
  1. try expressions with inline catch
  2. catch arrow expressions without block.

Those two should be seperate proposals.

@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.

@VBAndCs
Copy link

VBAndCs commented Mar 12, 2019

I would also propose two other try expressions:
1- value = try (expr1, expr1);
try returns the value of expr1 if it succeeded, or retirns the value of expr2 if expr1 throws exception)

2- bool success = try (expr);
try returns false if the expr throws an exception.
ex:

if try(var i = Convert.ToInt32(o))
   DoSomething(i);
else if try(var i = Convert.ToInt64(o))
   DoSomething(i);
else if try(var i = Convert.ToIntDecimal(o))
   DoSomething(i);
else if try(var i = Convert.ToDouble(o))
   DoSomething(i);

@DavidArno
Copy link

DavidArno commented Mar 12, 2019

@VBAndCs,

Why all the exception throwing? Just make use of pattern matching, nullable structs and the "try pattern":

if (TryConvertToInt(o) is int i)
    DoSomething(i);
else if (TryConvertToLong(o) is long l)
    DoSomething(l);
else if (TryConvertToDecimal(o) is decimal d)
    DoSomething(d);
else if (TryConvertToDouble(o) is double d2)
    DoSomething(d2);

This already works and doesn't require exception swallowing.

@HaloFour
Copy link
Contributor

@VBAndCs

using System;
using static Helpers;

public static class Helpers {
    public static bool Try<T>(Func<T> f, out T result) {
        try {
            result = f();
            return true;
        }
        catch (Exception) {
            result = default;
            return false;
        }
    }
}


static class Program {
    static void Main() {
        object o = null;
        if (Try(() => Convert.ToInt32(o), out var result)) {
            DoSomething(result);
        }
    }
}

@VBAndCs
Copy link

VBAndCs commented Mar 12, 2019

@HaloFour
Fascinating :)

@surfsky
Copy link

surfsky commented May 27, 2020

I like this syntax:

var stream = new StreamReader("C:\nonexistingfile.txt") !! null;

means

StreamReader stream;
try
{
    stream = new StreamReader("C:\nonexistingfile.txt");
}
catch
{
    stream = null;
}

see #3495

@HaloFour The under codes can change trycatch code to tranditional c-like code;

var o = "12text";
var result = Convert.ToInt32(0) !! -1;
if (result != -1)
    DoSomeThing(result);

In some cases, exception has been abused in some language(maybe java), and so rust choose the other way. How to design a API when matching unexpected parameter? Throwing exception maybe is a lazy way.

@Thaina
Copy link

Thaina commented May 27, 2020

@surfsky !! is already a valid syntax. ! could be overload to convert variable to boolean, such as in unity3d system

GameObject gameObject;
var boolean = !!gameObject; // intentionally boolean

@surfsky
Copy link

surfsky commented May 27, 2020

@Thaina Are you sure? I test under codes in netcore console, and get failure:

static void Main(string[] args)
{
     var o = "hello world";
     var b = !!o;  // CS0023 C# Operator '!' cannot be applied to operand of type 'string'
}

@surfsky
Copy link

surfsky commented May 29, 2020

@Thaina
I got it. !!boolvalue is bool operation.

@Thaina
Copy link

Thaina commented May 29, 2020

@surfsky Yeah, what I try to tell is, while !!boolValue actually do nothing so no one actually write that, !!someObject that has override ! is possible. especially if it was overridden to return bool. Such as unity GameObject class

!gameObject return true for null or destroyed GameObject. !!gameObject then being used to switch to false, to get state of existence of the gameObject as boolean

Another possibility such as, there would be 3 state rotation of value. And some people use ! to make switch from one state to the next. !! Let they skip to 3rd state

So as I said, it is already a valid syntax and some people might already use it for something we don't know of

@wangyoutian
Copy link

wangyoutian commented Nov 14, 2020

I'm in favor of this proposal, due to the following use case recently encountered by me that thence lands me here after searching:

I tried to fetch a configuration value with fall backs at different stage. Now my code looks like this:

int getSettings(int? callerGiven=null){
        return  callerGiven
		??
		(
			(Func<int?>)(
				() => {
					try{
								return	Properties.Settings.Default?.someSetting1;
					}
					catch (Exception)
					{
						return null;
					}
				}
			)
		)()
		??
		(int)Properties.Settings.Default.Properties[nameof(Properties.Settings.Default.someSetting1)].DefaultValue
      ;
}

//you can see the logic is simple (just fall back cascadingly ) but the code obliterates that simpleness, whileas with the proposed syntax somewhere in this discussion, we can revert that simpleness.

@theunrepentantgeek
Copy link

@wangyoutian this discussion is closed - if you read back through the threads you can see the compelling reasons why.

It's worth pointing out that you can easily restructure the sample you gave to make it much easier to read (and to reduce the number of allocations as well), just by breaking out each branch as a local function with a well chosen name:

int GetSettings(int? callerGiven = null)
{
    return  callerGiven
        ?? DefaultFromSettings()
        ?? DefaultValueFromSettings()
        
    int? DefaultFromSettings() 
    {
        try
        {
            return  Properties.Settings.Default?.someSetting1;
        }
        catch (Exception)
        {
            return null;
        }
    }

    int DefaultFromSettings() 
        => (int)Properties.Settings.Default.Properties[nameof(Properties.Settings.Default.someSetting1)].DefaultValue;
}

The names I chose for each function might not be the best, but I hope this illustrates my point.

@Xyncgas
Copy link

Xyncgas commented Feb 21, 2022

catch { } is evil. There are times when it might make sense to use it after careful analysis of the consequences. Having the compiler encourage its casual use through a syntactic shortcut though, would be a seriously bad thing to do.

But I know what I am doing, that's why it's a non breaking change for me to opt in

@theunrepentantgeek
Copy link

If something becomes enshrined in the C# language, that's an explicit endorsement that the approach is a good one.

The language design team have been quiet explicit about this - if it's wordy and difficult to write atrocious code, that's a good thing. Making it easier is not desirable.

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