40

I have one method that looks like this:

void throwException(string msg)
{
    throw new MyException(msg);
}

Now if I write

int foo(int x, y)
{
    if (y == 0)
        throwException("Doh!");
    else
        return x/y;
}

the compiler will complain about foo that "not all paths return a value".

Is there an attribute I can add to throwException to avoid that ? Something like:

[NeverReturns]
void throwException(string msg)
{
    throw new MyException(msg);
}

I'm afraid custom attributes won't do, because for my purpose I'd need the cooperation of the compiler.

user192472
  • 715
  • 7
  • 15
  • Other solutions would be acceptable (like a special return type ?), as long as they allow the code of `foo` to stay unchanged, and provided they are not too complex. – user192472 Jan 04 '10 at 12:18
  • TO MAKE THINGS CLEAR: the code above is a dumbed-down example. `throwException` will add contextual information to the exception. In `foo`, the `throwException` may appear inside a list of `if ... else if` cases, and some of them actually return meaningful values. – user192472 Jan 04 '10 at 13:14
  • 6
    I wish there was such a return type distinct from void -- "never" was the proposal for ECMAScript v4 -- but there is not. – Eric Lippert Jan 04 '10 at 14:47
  • 2
    http://blogs.msdn.com/b/ericlippert/archive/2011/02/21/never-say-never-part-one.aspx – Emperor XLII Feb 25 '11 at 23:20
  • It sounds like you want to either return or throw an exception from foo. The compiler can't figure out that's what you want to do, so it complains. Instead of a "never returns" attribute (which probably you want mean "always throws" in the example above), consider throwing straight from foo. Either return an exception from the method call, or wrap the contextual info you want in a custom exception class, and throw that. – Frank Bryce Jun 29 '15 at 20:09
  • Does this answer your question? [declare a method always throws an exception?](https://stackoverflow.com/questions/3892938/declare-a-method-always-throws-an-exception) – MuiBienCarlota Dec 04 '19 at 13:53
  • The updated article can be found at https://ericlippert.com/2011/02/21/never-say-never-part-one/ . – molnarm Jan 23 '23 at 09:44

10 Answers10

33

Why not just change it to

int foo(int x, y)
{
    if (y == 0)
        throwException("Doh!");
    return x/y;
}

This gives the same runtime results, and the compiler won't complain.

bernhof
  • 6,219
  • 2
  • 45
  • 71
27

No. I suggest you change the signature of your first function to return the exception rather than throw it, and leave the throw statement in your second function. That'll keep the compiler happy, and smells less bad as well.

Edit: there is now a DoesNotReturn attribute that provides an alternative.

David M
  • 71,481
  • 13
  • 158
  • 186
  • 5
    That's good practice? I've never seen anything done with an Exception object except throwing it. – Aviad P. Jan 04 '10 at 12:19
  • 1
    @Aviad P. No? Take a look here as to why that might be a good idea: http://stackoverflow.com/questions/1980044/when-should-i-use-a-throwhelper-method-instead-of-throwing-directly/1980078#1980078 – Mark Seemann Jan 04 '10 at 12:22
  • @Aviad P. - I said smells less bad, not smells good. Check out the question Mark linked to. – David M Jan 04 '10 at 12:25
  • 2
    I'd say it's a good practice if you need to build the exception in a more complex way, like wrapping it and adding additional information. The method should be renamed. I can't see any point in having a method that just throws an exception. – PHeiberg Jan 04 '10 at 12:25
  • 5
    I would call this an exception factory, this doesn't smell bad at all. – Stefan Steinegger Jan 04 '10 at 12:53
  • Much thanks for this answer. It will be my accepted answer once I understand how to mark an answer as accepted ;-) – user192472 Jan 04 '10 at 13:12
  • I think there's a tick next to it you can click on. – David M Jan 04 '10 at 13:16
  • I ended up flagging Christian Hayter's answer as accepted instead of yours, only because of his additional insight "functional code is better". Much thanks anyway. – user192472 Jan 04 '10 at 13:41
  • Change my mind once more as you're the only one (of the two) to have explicitly answered my question about the attribute ;-) Now back to real work. – user192472 Jan 04 '10 at 13:50
  • Returning an exception is a common practice actually; usually when you care about method inlining, JIT, and constructing your specialized exceptions in one place. – Jakub Míšek May 17 '21 at 11:05
  • This answer is out of date; as mentioned in another answer, [DoesNotReturn] now exists. – Reilly Wood Mar 21 '22 at 23:21
12

Bernhof's answer is correct. However, if you are trying to encapsulate a large chunk of logic when instantiating your exception, then all you need to do is change your code from this:

void throwException(string msg) {
    throw new MyException(msg);
}

to this:

Exception makeException(string msg) {
    return new MyException(msg);
}

Then your calling code will look like this:

int foo(int x, y) {
    if (y == 0) {
        throw makeException("Doh!");
    }
    return x / y;
}

All other things being equal, prefer functional code to procedural code. It's easier to re-use and unit-test.

EDIT:

In light of Fred's sample code, this is what I would do. It's not a code contract, but it's still functional.

private int getVarID(string s_varID) {
    int varID;
    if(s_varID == "ILT") {
        return 123;
    } else if(s_varID == "TL") {
        return 456;
    } else if(s_varID == "FT") {
        return 789;
    } else if(int.TryParse(s_varID, out varID)) {
        return varID;
    } else {
        throw makeParseError("varID must be an integer or 'ILT', 'TL' or 'FT'.");
    }
}
Christian Hayter
  • 30,581
  • 6
  • 72
  • 99
  • Much thanks for your answer. Almost got to be accepted. But there can be at most one accepted answer... Thanks also to remind me that "prefer functional code to procedural code. It's easier to re-use and unit-test", though in this case it won't change much (the throw/makeException method will stay short.) – user192472 Jan 04 '10 at 13:56
  • with the new C#8 switch syntax, the last code snippet (with `if else if ...` can be nicely rewritten, making the "functional version" even more practical. – Pac0 Mar 10 '20 at 14:03
12

The [DoesNotReturn] attribute in System.Diagnostics.CodeAnalysis should be what you want.

Reference: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.doesnotreturnattribute?view=netcore-3.1

double-beep
  • 5,031
  • 17
  • 33
  • 41
MBentley
  • 995
  • 10
  • 16
7

You can indicate "never returns" by using generics to declare that the function returns "anything":

T ThrowException<T>(string msg)
{
    throw new MyException(msg);
}

So now you can write:

int foo(int x, int y)
{
    if (y == 0)
        return ThrowException<int>("Doh!");
    else
        return x/y;
}

This idiom is used in languages like Haskell and F#, and is based on the principle of explosion, also known as "ex falso quodlibet". The reasoning is this: if a function never returns, then we can make whatever magical assumptions we want about its return value, since such value will never exist. Here, the caller (foo) assumes ThrowException will return an int.

A few minor drawbacks:

  • The implementation of ThrowException can circumvent this by returning default(T).
  • You have to specify the return type when calling ThrowException (Haskell and F# can infer it).
  • This idiom is very uncommon in C#, so many people won't recognize it. You may have to add a comment saying what you're doing.

As the other answers say, you're probably better off returning the exception rather than throwing it.

Joey Adams
  • 41,996
  • 18
  • 86
  • 115
  • Oh, this is kind of cool, because it means I can throw exceptions from expressions now, like in: ` static T DivZ() { throw new DivideByZeroException(); } static int Div(int x, int y) { return (y == 0) ? DivZ() : x / y; } ` – Nordic Mainframe Feb 22 '18 at 10:01
  • Alternatively: `static dynamic DivZ() { throw new DivideByZeroException(); } static int Div(int x, int y) { return (y == 0) ? DivZ() : x / y; }` – Nordic Mainframe Feb 22 '18 at 10:17
4

Don't hand the exception creation off to another function (i.e. just throw it directly) and the compiler won't complain. Handing off to a "helper" type function for exception throwing is a waste of time unless the function is actually adding value to the exception process.

slugster
  • 49,403
  • 14
  • 95
  • 145
  • Correct. So in this case a simple `throw new MyException("Doh!");` would be in order. I usually only use such throwing methods when there is some logic involved; the (often validation) method could throw on some specific condition with a specific message or it could effectively do nothing. – peSHIr Jan 04 '10 at 12:39
  • Well, my example was just dumbed-down code. In the actual code, `throwException` adds context information to the exception. – user192472 Jan 04 '10 at 13:06
  • 2
    It's useless in THIS case but I've hit this sort of thing many times when a variety of paths can hit the same exception logic. – Loren Pechtel Mar 27 '12 at 05:29
2

Your function

void throwException(string msg)
{
    throw new MyException(msg);
}

add zero value to the code, hence your question is moot. If, on the other hand you want to throw an error with the same message throughout the class and minimise code duplication this is what you should do.

The normal practice would be to extend MyException for this particular case and throw that:

public class HomerSimpsonException : MyException
{
   public HomerSimpsonException() : base ("DOH!!!"){
   }
}
int foo(int x, y)
{
    if (y == 0)
        throw new HomerSimpsonException();
    else
        return x/y;
}

Even then, that's not complete enough as per Microsoft rule for extending exceptions, there are minimum 4 constructors that you should implement - http://msdn.microsoft.com/en-us/library/ms182151%28VS.80%29.aspx, namely:

  public NewException(){}
  public NewException(string){}
  public NewException(string, Exception){}
  protected or private NewException(SerializationInfo, StreamingContext){}
Igor Zevaka
  • 74,528
  • 26
  • 112
  • 128
1

Bernhof already gave you a way to avoid the compiler complain. However, also be aware your stack trace will be off (and some logger libraries won't handle util-classed-i-throw-exceptions-for-your-app methods) which makes debugging your application harder.

Rick
  • 3,361
  • 1
  • 22
  • 29
  • Thanks for the insight. Here it will be less of a problem I guess because the 2 functions belong to the same class. Would David M's solution (exception created in separate function and thrown in `foo`) avoid the problem ? – user192472 Jan 04 '10 at 13:27
  • @fred-hh (answering myself) yes (this is documented). – user192472 Mar 04 '10 at 12:30
1

Remove the 'else' keyword, it's redundant anyway, and it will work ;)

Fedor Hajdu
  • 4,657
  • 3
  • 32
  • 50
1

Well, this is a "pretty" low-effort implementation.

Working class:

/// <summary>
/// Representation of an unreachable type, exposing a method to represent unreachable code.
/// </summary>
public static class Unreachable {

    /// <summary>
    /// Representation of unreachable code with return semantics.
    /// </summary>
    public static dynamic Code() {
        throw new NotImplementedException(@"Unreachable code was reached.");
    }
}

Example:

public object[] UnreachableCodeTest() {
    return Unreachable.Code();
}

Decompiled:

Offset  OpCode  Operand
0   ldsfld  System.Runtime.CompilerServices.CallSite`1<System.Func`3<System.Runtime.CompilerServices.CallSite,System.Object,System.Object[]>> TestApp.Program/<UnreachableCodeTest>o__SiteContainere6::<>p__Sitee7
5   brtrue.s    -> (10) ldsfld System.Runtime.CompilerServices.CallSite`1<System.Func`3<System.Runtime.CompilerServices.CallSite,System.Object,System.Object[]>> TestApp.Program/<UnreachableCodeTest>o__SiteContainere6::<>p__Sitee7
7   ldc.i4.0    
8   ldtoken System.Object[]
13  call    System.Type System.Type::GetTypeFromHandle(System.RuntimeTypeHandle)
18  ldtoken TestApp.Program
23  call    System.Type System.Type::GetTypeFromHandle(System.RuntimeTypeHandle)
28  call    System.Runtime.CompilerServices.CallSiteBinder Microsoft.CSharp.RuntimeBinder.Binder::Convert(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags,System.Type,System.Type)
33  call    System.Runtime.CompilerServices.CallSite`1<!0> System.Runtime.CompilerServices.CallSite`1<System.Func`3<System.Runtime.CompilerServices.CallSite,System.Object,System.Object[]>>::Create(System.Runtime.CompilerServices.CallSiteBinder)
38  stsfld  System.Runtime.CompilerServices.CallSite`1<System.Func`3<System.Runtime.CompilerServices.CallSite,System.Object,System.Object[]>> TestApp.Program/<UnreachableCodeTest>o__SiteContainere6::<>p__Sitee7
43  ldsfld  System.Runtime.CompilerServices.CallSite`1<System.Func`3<System.Runtime.CompilerServices.CallSite,System.Object,System.Object[]>> TestApp.Program/<UnreachableCodeTest>o__SiteContainere6::<>p__Sitee7
48  ldfld   !0 System.Runtime.CompilerServices.CallSite`1<System.Func`3<System.Runtime.CompilerServices.CallSite,System.Object,System.Object[]>>::Target
53  ldsfld  System.Runtime.CompilerServices.CallSite`1<System.Func`3<System.Runtime.CompilerServices.CallSite,System.Object,System.Object[]>> TestApp.Program/<UnreachableCodeTest>o__SiteContainere6::<>p__Sitee7
58  call    System.Object TestApp.Unreachable::Code()
63  callvirt    !2 System.Func`3<System.Runtime.CompilerServices.CallSite,System.Object,System.Object[]>::Invoke(!0,!1)
68  ret 

Optimize by implementing something w/ Fody, searching for this call signature and replacing it with a single raw ret (if it'll let you) or some other simple acceptable low-cost alternative.

(Edit)

As you mod me down, I will be forced to explain why this is effective and useful.

Mostly, this goes into a block that resembles at the end of a method that has a logically unreachable path;

#pragma warning disable 162
// ReSharper disable CSharpWarnings::CS0162
// ReSharper disable HeuristicUnreachableCode
return Unreachable.Code();
// ReSharper restore HeuristicUnreachableCode
// ReSharper restore CSharpWarnings::CS0162
#pragma warning restore 162

If you modify the code above this section in a way that allows the code to be reached, your tests will fail. If you have code below, you're wrong and the compiler will let you know. Once you move beyond initial maturity, you would remove this code block all together. The main purpose of this is to catch scenarios where the code is not unreachable when it should be.

In other scenarios where code should not arrive but can't be logically excluded by the compiler (a default statement in a case, for example), you would traditionally throw an error. If you want to optimize for this scenario, you need this sort of implementation.

select( thisIsAlways123Never0OrGreaterThan3 ) {
    default: return Unreachable.Code();
    case 1: DoSomething(); break;
    case 2: DoSomethingElse(); break;
    case 3: return GetSomething();
}

To optimize such that minimal instructions are emitted for the default: path with this minimally written code, you need a friend named Fody.

The ideal scenario is a result similar to what C++ developers desire in the GCC & LLVM __builtin_unreachable() or MSVC __assume(0) or __declspec(noreturn) on an empty method.

TylerY86
  • 3,737
  • 16
  • 29