Reputation: 745
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.
Upvotes: 43
Views: 7224
Reputation: 72930
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.
Upvotes: 29
Reputation: 1016
The [DoesNotReturn]
attribute in System.Diagnostics.CodeAnalysis
should be what you want.
Upvotes: 13
Reputation: 3792
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.
Upvotes: 1
Reputation: 43410
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:
ThrowException
can circumvent this by returning default(T)
.ThrowException
(Haskell and F# can infer it).As the other answers say, you're probably better off returning the exception rather than throwing it.
Upvotes: 7
Reputation: 31071
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'.");
}
}
Upvotes: 12
Reputation: 76600
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){}
Upvotes: 2
Reputation: 4695
Remove the 'else' keyword, it's redundant anyway, and it will work ;)
Upvotes: 1
Reputation: 3451
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.
Upvotes: 1
Reputation: 49965
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.
Upvotes: 4
Reputation: 6330
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.
Upvotes: 33