What should I do when I am forced to write unreachable code?

Question

I have this simple piece of code:

public static int GetInt(int number)
{
    int[] ints = new int[]{ 3, 7, 9, int.MaxValue };
    foreach (int i in ints)
        if (number <= i)
            return i;

    return int.MaxValue; //this should be unreachable code since the last int is int.MaxValue and number <= int.MaxValue is allways true so the above code will allways return
}

The problem is that the compiler says that not every execution path returns a value. So I have to write code that will be never reached. My question is, what should I do in a situation like this? Should I return some default value or should I throw an exception. Also, if I want to throw an exception, what exception is suitable for throwing? I didn't find anything like UnreachableCodeException.

Alric

You can declare i out of the foreach and halt on condition number <= i with a break, and then you have only one return – Neurogenic

Answer 1

I'd be tempted to use InvalidOperationException - or some other exception which you wouldn't explicitly catch. Give it a message which indicates that you really didn't expect to get here. This is a "world is seriously broken" failure. InvalidOperationException doesn't quite capture this, but I can't think of a better one offhand. You could always create your own exception to use throughout your codebase, of course.

Don't just return a value, as otherwise you'll never find out if your world is upside-down.

Colligate

So there is no built-in .NET exception for unreachable code? I'm not so sure about using InvalidOperationException. Isn't there any logic failure exception or something more close to the problem? – Alric

On a secound tought InvalidOperationException is good when I saw what IEnumerable.First() throws InvalidOperationException when no item is found. – Alric

@Bosak: Well, InvalidOperationException makes more sense for First than it does for your case. That really is calling a method when it's not in a valid state - in your case, it's something you really, really don't think should happen. (Assuming you already know that the array is non-empty and contains int.MaxValue.) – Colligate

NotImplementedException looks a good candidate since it's never meant to be caught or expected in production code. – Archival

@ivan_pozdeev: But to me that usually suggests that there's a code path which could reach it, but we've not implemented that functionality yet. – Colligate

@JonSkeet see the edit that puts stress on "production code". Besides, there is logic in this: if a program got into such a situation, then something had happened that there are no (implemented) means to handle. (maybe, say, another member had been added to an enum) – Archival

@ivan_pozdeev: I agree that it shouldn't be in production code - which means I'd be surprised to see it in production-ready source code. It would make me think that it had slipped past code review. I still think InvalidOperationException with a suitable message is clearer, but it really is a matter of opinion. – Colligate

Answer 2

.NET 7 introduced the new UnreachableException class for this purpose, as I just learned by watching the YouTube video The new .NET Exception that should NEVER be thrown by Nick Chapsas.

throw new UnreachableException();

Manufacturer

lol just watched that video too! – Unused

Answer 3

Use the following to show a logic failure message after your foreach:

System.Diagnostics.Debug.Fail("Unreachable code reached");

This will alert you during debugging.

In addition, also throw an exception for during production:

throw new InvalidOperationException();

Do not just return a value, especially one that is potentially valid: you'll never catch the logic error.

Boughten

Using both Debug.Fail and an exception is a good idea; this calls out that the exception is not intended to actually be an executable code path. However I'd choose a better exception than new Exception. Jon suggests InvalidOperationException which seems reasonable. – Foreigner

Answer 4

I think every case is different, but yes, ultimately you have to return something or throw an exception. The way to handle this in your code example is just to remove int.MaxValue from your array:

public static int GetInt(int number)
{
    int[] ints = new int[]{ 3, 7, 9 };
    foreach (int i in ints)
        if (number <= i)
            return i;
    return int.MaxValue;
}

Collimate

the situation is a little bit more complex and I can not modify the collection – Alric

Answer 5

Rather than returning from your loop, declare a return value variable, set it, and then return once at the end of the code.

public static int GetInt(int number)
{
    var rtnVal = int.MaxValue;
    int[] ints = new int[]{ 3, 7, 9, int.MaxValue };
    foreach (int i in ints) {
        if (number <= i) {
            rtnVal = i;
            break;
        }
    }
    return rtnVal;
}

Disparagement

Answer 6

Here's a LINQ option that automatically throws an exception when no match is found

public static int GetInt(int number)
{
    int[] ints = new int[]{ 3, 7, 9, int.MaxValue };
    return ints.First(i => number <= i);
}

Collimate

Hmm nice I like how simetyc i => number <= i looks :D – Alric

Answer 7

The compiler cannot tell that your foreach loop will always return a value.

A theoretical compiler could do so since in principal the information is available, but the C# compiler cannot.

Umbles

Answer 8

Why not just return the first value that is greater than number instead

    public static int GetInt(int number)
    {
        var ints = new[] { 3, 7, 9};
        return (ints.Any(i => i > number))? 
            ints.First(i => i > number): int.MaxValue;
    }

Chilpancingo

Answer 9

In .NET 7 UnreachableException has been added see the answer by Theodor Zoulias above.

Before .NET 7, I think the best solution is to copy the code of the UnreachableException class into you project, and when you update to .NET 7 you can remove this class, without changing any line used by this class.

Judiciary