20

I find myself writing some methods where there is a code path that should never happen. Here is a simplified example:

double Foo(double x) {
    int maxInput = 100000;
    double castMaxInput = (double)maxInput;
    if (x < 0 || x > castMaxInput || double.IsNaN(x)) {
        return double.NaN;
    }
    double r = 0;
    for (double boundary = 1; boundary<=castMaxInput; boundary++) {
        if (x <= boundary) {
            r += boundary * (x + 1 - boundary);
            return r;
        }
        else {
            r += boundary;
        }
    }

    // we should never get here.
    throw new SomeException();
}

The exception that would make the most sense here is something like

TheAuthorOfThisMethodScrewedUpException() 

Because that's what is going on if we reach the end of the for loop. Unfortunately, with the method structured as above, the compiler does not appear to be smart enough to figure out that the code after the for loop should never happen. So you can't just have nothing there, or the compiler will complain that "not all code paths return a value". Yes, I could put in return double.NaN after the loop in addition to before it. But that would disguise the source of the problem.

My question is – is there an exception that would be appropriate?

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
William Jockusch
  • 26,513
  • 49
  • 182
  • 323

5 Answers5

9

I use the InvalidOperationException class for that. It means that the application has reached a state it should not be in.

throw new InvalidOperationException("Invalid state.");

You can also Debug.Assert that something is true, or simply Debug.Fail when execution reaches a particular point.

Debug.Fail("This should never happen!");

But debugging asserts/fails don't work in release mode, only when the DEBUG conditional is defined. Depends on your requirements whether that's desirable.

As @AlexD correctly points out, there's also the Trace class with its corresponding Assert and Fail methods, that will work at run-time to help isolate and fix problems without disturbing a running system, when the TRACE conditional is defined (is set by default in the Project Properties Build tab).


By the way, to answer the question in the title: you can create your own exceptions if you want.

[Serializable]
public class TheAuthorOfThisMethodScrewedUpException : InvalidOperationException
{
    private const string DefaultMessage = "The author of this method screwed up!";

    public TheAuthorOfThisMethodScrewedUpException()
        : this(DefaultMessage, null)
    { }

    public TheAuthorOfThisMethodScrewedUpException(Exception inner)
        : base(DefaultMessage, inner)
    { }

    public TheAuthorOfThisMethodScrewedUpException(string message)
        : this(message, null)
    { }

    public TheAuthorOfThisMethodScrewedUpException(string message, Exception inner)
        : base(message, inner)
    { }

    protected TheAuthorOfThisMethodScrewedUpException(
      System.Runtime.Serialization.SerializationInfo info,
      System.Runtime.Serialization.StreamingContext context)
        : base(info, context)
    { }
}

And throw it at people.

throw new TheAuthorOfThisMethodScrewedUpException();
Daniel A.A. Pelsmaeker
  • 47,471
  • 20
  • 111
  • 157
  • You could use `Trace.Assert` / `Trace.Fail` in release mode (assuming that `TRACE` is defined). – AlexD Feb 18 '14 at 18:40
4

As of .NET 7, the UnreachableException should be used.

sasha_gud
  • 1,635
  • 13
  • 18
2

Don't throw new Exception(), it causes problems for code trying to catch exceptions. The generic, specific exception that you can use is:

throw new InvalidOperationException("Appplication invariants violated");

This assumes you want the error to happen in production, assuming that an error is better than launching missiles and ending the world. Other developers would rather use a method that assumes the invariant can be ignored in production but not at development time and we don't care if we end the world or not.

MatthewMartin
  • 32,326
  • 33
  • 105
  • 164
  • According to MSDN, `InvalidOpertionException` means "*The exception that is thrown when a method call is invalid for the object's current state.*" This is not exactly our case. – AlexD Feb 18 '14 at 18:22
  • It's used as a catch-all. For the above case, the method call is invalid for all states because the code is defective/no longer trusted by the time it reaches that point. AFAIK, there aren't any other catch-all exceptions in the BCL that MSDN doesn't discourage application writers from using. – MatthewMartin Feb 18 '14 at 22:37
  • Well, I also do not know a good alternative for a general "Should-not-get-here" exception, and looking for it also considered `InvalidOperationException`. But still, I'm not completely happy with semantics. `InvalidOperationException` looks more appropriate to report *misuse by client* (the client called a method without fulfilling preconditions), and not to report *internal errors*, like in our case. – AlexD Feb 18 '14 at 22:46
1

It looks easy to create a Custom Exception

public class TheAuthorOfThisMethodScrewedUpException: Exception
{
    public EmployeeListNotFoundException()
    {
    }

    public EmployeeListNotFoundException(string message)
        : base(message)
    {
    }

    public EmployeeListNotFoundException(string message, Exception inner)
        : base(message, inner)
    {
    }
}

then

throw new TheAuthorOfThisMethodScrewedUpException("I am so sorry, this should never happen call me for more info")
bto.rdz
  • 6,636
  • 4
  • 35
  • 52
0

Easy! Use the code snippet!

Exception + TAB +TAB

And it will create a new exception for you. This snippet produces this.

[Serializable]
    public class MyException : Exception
    {
        public MyException() { }
        public MyException(string message) : base(message) { }
        public MyException(string message, Exception inner) : base(message, inner) { }
        protected MyException(
          System.Runtime.Serialization.SerializationInfo info,
          System.Runtime.Serialization.StreamingContext context)
            : base(info, context) { }
    }

You only need to change the name and.. done! ;)

Oscar Bralo
  • 1,912
  • 13
  • 12