How to handle "not all code paths return a value" when the logic of the function does ensure a return

Question

I suppose the easiest way to explain is by a contrived example:

public static int Fail() {
    var b = true;
    if (b) {
        return 0;
    }
}

This code will not compile, and gives the error "not all code paths return a value," while we humans can clearly see that it does. I DO understand why. My question is what should be done to remedy the situation. It could be something like this:

public static int Fail() {
    var b = true;
    if (b) {
        return 0;
    }
    throw new ApplicationException("This code is unreachable... but here we are.");
}

But it all just seems rather silly. Is there a better way? Again, this code is a contrived example (and can be reduced to return 0). My actual code is massive and complex, but does logically (by mathematical proof) return a value before trying to exit.

Answer 1

C# code flow analysis is limited and as your sample points out there are cases where all paths do return but the compiler can't detect it. Throwing an exception is an acceptable remedy in those circumstances.

I would not use a return of a default value to fix this error. You're operating under the assumption that the line is never hit against the advice of the compiler. Consider for a second that your analysis is wrong and execution can proceed to the end of the method. If you return a default value then you will have no indication of the problem. The method will just be returning bad data. Throwing an exception will make it very obvious that there is a problem.

However in those cases my preference is to simply rewrite the code such that the compiler can see that all paths terminate. In general I've found that if the method is so complex that the compiler can't follow it then the guy who picks up the code after me won't be able o follow it either.

Answer 2

I think your current approach is correct. Returning a default value at the end of the method is risky, because if there's a flow in your logic, this instruction could be reached, which could have unintended consequences. If you throw an exception, it gives you a chance to detect the mistake.

Answer 3

The compiler doesn't perform complex mathematical processes to ensure you return a value, it's a relatively dumb beast in that regard.

If you're certain of your assumptions, just add return 0; at the end with a comment stating the reality, that it will never be reached.

Of course, any code where it can be guaranteed the final condition will be true can just have the condition removed, as you've already stated. That's mathematically true even for complex cases. So you could refactor your code to take that into account without a superfluous return.

Answer 4

There are usually a couple of classes of programmers:

1. They who return all over.
2. They who return only at the end of a funcion/method.

First approach:

public static int Fail() {
  var b = true;
  if (b)
    return 0;

  return 1;
}

I usually prefer the second approach since it's easier to quickly see where a function/method returns.

public static int Fail() {
  var b = true;
  var returnValue = 1:
  if (b)
    returnValue = 0;

  return returnValue;
}

Answer 5

Really late answer, but I can imagine a scenario in which the function will not know what to return:

1) Run step by step using the debugger

2) Step over var b = true;

3) When if (b) { is reached, put b = false in the watch. It will both return false and assign b = false

4) Step over (over if)

5) End of function is reached, no return is there

From this perspective, return is clearly needed.

Answer 6

I had code like this and fixed it this way. Looking at your example:

public static int Fail() {
    var b = true;
    if (b) {
        return 0;
    }
    throw new ApplicationException("This code is unreachable... but here we are.");
}

would become

public static int Fail() {
    var b = true;
    if (!b) {
        throw new ApplicationException("This code is unreachable... but here we are.");
    }
    return 0;
}

This will get rid of the compiler warning, still be logically the same, and actually be slightly cleaner code.