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

Catch multiple exceptions instead of when keyword #11889

Closed
deepumi opened this issue Jun 9, 2016 · 15 comments
Closed

Catch multiple exceptions instead of when keyword #11889

deepumi opened this issue Jun 9, 2016 · 15 comments

Comments

@deepumi
Copy link

deepumi commented Jun 9, 2016

Handle multiple exceptions in C# we need to use when keyword, So In C# 7.0 can we please simplify this like below.

Expected Behavior:

try
{
}
catch(FormatException || OverflowException ex)
{
}

Actual Behavior:

catch(Exception ex) when (ex is FormatException || ex is OverflowException ex)
{
}
@HaloFour
Copy link

HaloFour commented Jun 9, 2016

I think that this would be useful and help to avoid duplicating code or unnecessary casts.

To expand on this it would be nice to allow for the common members between the unioned types to be referenced, e.g.:

class Exception1 : Exception {
    string Foo { get; }
}

class Exception2 : Exception {
    string Foo { get; }
}

// and then

try { ...
}
catch (Exception1 || Exception2 exception) {
    var foo = exception.Foo; 
}

@realbart
Copy link

realbart commented Jun 9, 2016

try
{
  catch(FormatException || OverflowException ex)
}

What would be the type of ex?
I can imagine that FormatException needs to be able to castable to OverflowException, or the other way around. I'd exprect the behavior to be similar to:

var ex = booleanCondtion ? new FormatException() : new OverflowException();   

(this yields a compiler error)
If ex just returns a general Exception, I fear many the advantages of throwing and catching typed exceptions are gone.

Edit
Halo4 proposes a form of duck typing: the exception is either returned as dynamic or as a compiler generated combined type. I'm not sure I like the additional complexity this generates...

@iam3yal
Copy link

iam3yal commented Jun 9, 2016

@realbart Because C# lacks unions then maybe the type can always be the base class like this:

abstract class MyException
{
    public abstract string Foo { get; }
}

class Exception1 : MyException
{
    public override string Foo { get; }
}

class Exception2 : MyException
{
    public override string Foo { get; }
}

So in the case of @HaloFour the type would be MyException but in most cases it would be Exception.

Same idea just a little different, using inheritance.

@HaloFour
Copy link

HaloFour commented Jun 9, 2016

Oops, I swore that Java did allow referencing the common members, but that's not the case. It treats the exception as the common ancestor of the unioned exception types, and you can access any of those members.

I think that it would be possible to allow referencing the common members, but it would require a bit of compiler chicanery. The compiler could duplicate the generated IL, or it could upcast to one of several potential locals:

try { ... }
catch (Exception1 || Exception2 exception) {
    Type type = exception.GetType();
    var foo = exception.Foo;
}

// transpiled into:

try { ... }
catch (Exception exception when (exception is Exception1 || exception is Exception2)) {
    Exception1 $temp1 = exception as Exception1;
    Exception2 $temp2 = exception as Exception2;

    Type type = exception.GetType();
    string foo = ($temp1 != null) ? $temp1.Foo : $temp2.Foo;
}

I think that it would be neat, but I wouldn't hold up this proposal for it.

@dotnetchris
Copy link

dotnetchris commented Jun 9, 2016

Just define ex as dynamic in catch body. We already suffered the huge penalty of the throw, what does it matter for several more cycles for all the DLR hoops.

Besides, the majority of people that would use this just want to catch a couple of similar-ish exceptions. I doubt anyone will really care whether ex in the original example is anything other than Exception.

So i'd even be fine if ex was just Exception. But i'd prefer dynamic over the base type.

I'm very opposed to some magic constructed/combined type. However typing this, theoretically ex could be AggregateException and from there you could care about the actual specifics.. or not.

@iam3yal
Copy link

iam3yal commented Jun 9, 2016

@dotnetchris dynamic isn't really great because people would get really confused about the fact that they don't get intellisense when they explicitly specified the types so it would be implicitly dynamic, I don't think that this is such a great idea.

I don't know if AggregateException is actually suitable here, because in this scenario only one exception is thrown at a time.

Using the base type makes more sense but it also adds more complexity so to do this right maybe C# needs union types first before they add this feature (or similar features that requires it) then it wouldn't matter because the type would be a union of multiple types.

@jerhon
Copy link

jerhon commented Jun 10, 2016

Why not loosen the restrictions and allow to allow single lines to fall through instead of the {} blocks? It would be a similar idea to a case statement. It has precedence in other blocks in the language, in using statements you can do something similar. For example, you could then do something like this to get the base exception using the ?? operator without having to complicate the meaning of the catch.

try 
{
// some code here...
}
catch (IOException io)
catch (Exception ex)
{
    var finalException = (io ?? ex);
}
finaly
{
// some code here...
}

@iam3yal
Copy link

iam3yal commented Jun 10, 2016

@jerhon how is it different than this?

try
{
    ThrowingMethod();
}
catch (Exception ex)
{
    var finalException = (ex as IOException) ?? (ex as FormatException) ?? ex;
}

@realbart
Copy link

realbart commented Jun 10, 2016

I find myself constantly checking for null.

try {}
catch (Exception1 ex1 || Exception2 ex2) 
{
    ex1?.Foo();
    ex2?.Foo();
    log(ex1??ex2).ToString()
}

try {}
catch (Exception1 ex1)
catch (Exception2 ex2) 
{
    ex1?.Foo();
    ex2?.Foo();
    log((ex1??ex2).ToString())
}

Perhaps it is easier to let ex simply be of the Exception type. You could find the nearest base class, but that would cut off the possibility to replace a base class with an interface. (If not, what interface would you pick?)

class Exception1 : IExceptionA, IExceptionB {...}
class Exception2 : IExceptionA, IExceptionB {...}

try {}
catch (Exception1 || Exception2 ex) // ex is of type Exception.
{
    ((IExceptionA)ex).Foo():
    log(ex.Message);
} 

However, I am not sure if this justifies additional syntax.
This is nog that much verbose, and Explicitly specifying the cought exception type does have its advantages.

try {}
catch (Exception ex) 
when (ex is Exception1 || ex is Exception2)
{
    ((IExceptionA)ex).Foo():
    log(ex.Message);
}

@iam3yal
Copy link

iam3yal commented Jun 10, 2016

@realbart Currently, no matter what the best type/option here is to fall to the base type which is Exception, as you said it yourself but in the future if C# will get union types then this experience is likely to be much better, I think that if we're going to solve problems that involves multiple types that are treated as a single heterogeneous type then we need to solve this problem first.

@jerhon
Copy link

jerhon commented Jun 10, 2016

@eyalsk It is not different than what you pointed out. Perhaps a better example would be,

try
{
}
catch (FormatException fe)
catch (OverflowException oe)
{
       var exception = fe ?? oe as Exception;
}

The advantage to this over the other approach is we could still fit in the when clause and have it apply to the specific exceptions types in those when clauses.

@realbart I agree my suggestion does lead to a lot of null checking which isn't good.

@HaloFour
Copy link

Fall-through wouldn't make sense. None of the identifiers would be definitely assigned and assigning null to them would be contrary to how C# normally behaves.

If I were to vote on a syntax/behavior now I would go for the original proposal, which is basically what Java supports:

try {
    doSomething();
}
catch (Exception1 | Exception2 exception) {
    // exception is of the common ancestor here
    exception.printStackTrace(System.err);
}

Personally I think that the whole concept should be put off until union types and/or pattern matching further progresses. Union types would make the above nicer by allowing access to common members, which couldn't be done after the fact as it would be a breaking change due to member shadowing. Pattern matching with an OR pattern could open up the possibilities further, perhaps allowing deconstruction directly in the catch clause.

@realbart
Copy link

class BaseException : Exception {...}
class Exception1 : BaseException, IExceptionA, IExceptionB {...}
class Exception1 : BaseException, IExceptionA, IExceptionB {...}

try {}
catch (Exception1 || Exception2 ex) 
{
    // I think ex should be of the Exception type
    // NOT of BaseException, IExceptionA or IExceptionB.
    // (Otherwise: why not just catch BaseException
    log(ex.Message);
}

@HaloFour
Copy link

@realbart

You wouldn't just catch BaseException because you wouldn't want Exception3 or any other exception class deriving from BaseException. You could, of course, use catch guards to prevent catching those types, e.g. catch (BaseException exception when exception is Exception1 || exception is Exception2), but that is the very syntax that this proposal is seeking to make less verbose.

@gafter
Copy link
Member

gafter commented Apr 28, 2017

Issue moved to dotnet/csharplang #519 via ZenHub

@gafter gafter closed this as completed Apr 28, 2017
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

8 participants