23

Which exception should I use when the program reaches a logic state that I "know" won't happen, and if it does, something is terribly bad?

For example:

int SomeFunction(int arg) {
    SomeEnum x = Whatever(arg, somePrivateMember);
    switch (x) {
        case SomeEnum.Value1:
            return SomeFunction1();
        case SomeEnum.Value1:
            return SomeFunction2();
        default:
            throw new WhatTypeToThrow();
    }
}

Clearly, ArgumentException is a long-shot here since the invalid value for x could have come from a bug in Whatever(), or an invalid combination of any arguments and/or the current instance state.

I'm looking for something such as an InvalidProgramStateException, InternalErrorException or similar.

Of course I could define my own, but I wonder if there is a suitable exception in the framework.

Edit: Removed the simple sample code to reduce amount of ArgumentException answers.

Pang
  • 9,564
  • 146
  • 81
  • 122
erikkallen
  • 33,800
  • 13
  • 85
  • 120
  • 1
    Why not create your own exception using one of the names that you already suggested? Or how about a `ShouldNotHappenException`? ;-) – Dirk Vollmar Jul 22 '10 at 22:21
  • 3
    @0xA3 that's not considered a best practice – John Saunders Jul 22 '10 at 23:09
  • 2
    @erik: you don't have an internal error here. You have an error in the argument being passed to your method. That may amount to an internal error in a global sense, but what you have at the local level is an `ArgumentOutOfRangeException`, as "chibacity" says below. – John Saunders Jul 22 '10 at 23:13

7 Answers7

6

What about InvalidOperationException?

  • 6
    I sometimes use that one, but I think that exception says "You cannot do this", and what I want to say is "You should have been able to do this, but there is a bug". – erikkallen Jul 22 '10 at 22:13
  • From [MSDN](https://msdn.microsoft.com/en-us/library/system.invalidoperationexception%28v=vs.110%29.aspx): "InvalidOperationException is used in cases when the failure to invoke a method is caused by reasons other than invalid arguments.". So, my vote goes to this alternative. – fernio May 06 '15 at 21:07
  • 1
    @erikkallen, maybe the NotImplementedException? – Fred Sep 03 '15 at 15:12
  • @fernio The link you provide says: `The exception that is thrown when a method call is invalid for the object's current state.` – vines Aug 17 '17 at 12:00
4

Why not the InvalidEnumArgumentException? It looks like it was designed specifically for this use-case.

David Keaveny
  • 3,904
  • 2
  • 38
  • 52
4

Don't throw any specific exception type in the code you're looking at. Call Trace.Assert, or in this case even Trace.Fail to get a similar effect to Debug.Assert, except enabled even in release builds (assuming the settings aren't changed).

If the default trace listener, the one that offers a UI that offers to kill the whole program or launch a debugger, isn't appropriate for your needs, set up a custom trace listener in Trace.Listeners that causes a private exception type to be thrown whenever Trace.Fail is called (including when a Trace.Assert fails).

The exception type should be a private exception type because otherwise, callers may be tempted to try catching whatever exception type you're going to throw. For this particular exception type, you will want to make it as clear as possible that a future version of the method will no longer throw this particular exception. You don't want to be forced to throw a TraceFailedException or whatever you call it from now until eternity to preserve backwards compatibility.


Another answer mentioned Code Contracts already as an alternative. It is, in a similar way: you can call Contract.Assert(false). This takes the same approach of making it customisable what happens if an assertion fails, but in this case, the default behaviour is to throw an exception, again of a type that is not externally accessible. To make the most of Code Contracts, however, you should be using the static rewriter, which has both pros and cons that I will not get into here. If for you the pros outweigh the cons, then by all means use it. If you prefer to avoid the static rewriter though, then I'd recommend avoiding the Contract class entirely, since it is not at all obvious which methods do and don't work.

2

I think ArgumentOutOfRangeException is valid here and it's what I use. It's the argument to the switch statement that is not handled as it's out of the range of handled values. I tend to code it like this, where the message tells it like it is:

switch (test)
{
    case SomeEnum.Woo:
        break;
    case SomeEnum.Yay:
        break;
    default:
    {
        string msg = string.Format("Value '{0}' for enum '{1}' is not handled.", 
            test, test.GetType().Name);

        throw new ArgumentOutOfRangeException(msg);
    }
}

Obviously the message is to your own tastes, but the basics are in that one. Adding the value of the enum to the message is useful not only to give detail concerning what known enum member was not handled, but also when there is an invalid enum i.e. the old "(666)SomeEnum" issue.

Value 'OhNoes' for enum 'SomeEnum' is not handled.

vs

Value '666' for enum 'SomeEnum' is not handled.

Tim Lloyd
  • 37,954
  • 10
  • 100
  • 130
  • 2
    What if the switched value is not an argument? Btw, I removed the simple sample where the answer was likely to be ArgumentException. – erikkallen Jul 23 '10 at 08:55
  • 2
    @erikkallen The value is an argument to the switch statement as I stated in my answer. – Tim Lloyd Jul 23 '10 at 11:14
1

Here are suggestions that I've been given:

  • ArgumentException: something is wrong with the value

  • ArgumentNullException: the argument is null while this is not allowed

  • ArgumentOutOfRangeException: the argument has a value outside of the valid range

Alternatively, derive your own exception class from ArgumentException.

An input is invalid if it is not valid at any time. While an input is unexpected if it is not valid for the current state of the system (for which InvalidOperationException is a reasonable choice in some situations).

See similar question and answer that I was given.

Community
  • 1
  • 1
Even Mien
  • 44,393
  • 43
  • 115
  • 119
  • 1
    The point of the question is that the invalid state is not related to the arguments. I agree, in the simple case, ArgumentException can do, but I don't think ArgumentException should be used as a generic "oops, didn't work, I bet it had to do with the arguments" exception. – erikkallen Jul 23 '10 at 08:54
  • The switch statement is what has the argument out of range. – Even Mien Jul 23 '10 at 17:34
1

program reaches a logic state that I "know" won't happen, and if it does, something is terribly bad.

In this case, I would throw an ApplicationException, log what you can, and exit the app. If things are that screwed up, you certainly shouldn't try to recover and/or continue.

Pang
  • 9,564
  • 146
  • 81
  • 122
automatic
  • 2,727
  • 3
  • 34
  • 31
  • 3
    I don't think this is a good idea. The original purpose of the ApplicationException (before it wes deprecated) was that any (expected) exception that your application throws should be derived from it. – erikkallen Jul 23 '10 at 08:53
1

You should consider using Code Contracts to not only throw exceptions in this case, but document what the failed assumption is, perhaps with a friendly message to the programmer. If you were lucky, the function you called (Whatever) would have a Contract.Ensures which would catch this error before you got your hands on it.

Sebastian Good
  • 6,310
  • 2
  • 33
  • 57