Unreachable Code Detected

Question

I'm about ready to deploy an MVC web application that I've been tasked with managing (I did not create the application). The project is now compiling in production mode with no errors, however I have some warnings - 9 to be precise.

Now 6 are to do with the test project which is fine, however there are two that involve the Web project. these errors are:

Unreachable code detected

In both instances these warnings are thrown on the Return value, e.g.

protected override ValidationResult IsValid(object value, ValidationContext validationContext)
{
    if (true)
    {
        return new ValidationResult("Passwords don't match", new string[] { OriginalProperty });
    }

    return null;
}

In the above example, the "return null" line throws the unreachable code warning.

This might be a silly question (so please go easy ;-) ), but how important are these warnings to the functionality of the application? Obviously they are there for a reason, but they're not errors, so would I be relatively okay to ignore them and deploy?

Answer 1

Your if condition always evaluates to true (if (true)) so this method is equivalent to:

protected override ValidationResult IsValid(object value, 
    ValidationContext validationContext)
{
    return new ValidationResult("Passwords don't match", 
        new string[] { OriginalProperty });
}

And that's why you get the compiler warning. The last line which returns null could never be hit. As far as ignoring warnings is concerned I would recommend you to never ignore them. There are cases where a warning could lead in an unexpected behavior during runtime. Personally I've checked the option in VS which treats warnings as errors to make sure never miss a warning.

Answer 2

What Darin said.

This particular warning won't affect execution time, performance or reliability. However, I try to get rid of warnings so that, in development, I don't have warning messages distracting me (as a developer) from other messages that might be important and relevant.

You could try putting the return null into a newly created else branch of your if. It will still never execute, but if you ever change true in the if to false, you'll still have correct code. You may also get rid of the warning that way.

Answer 3

You can safely delete the unreachable code without changing how the program currently works.

The problem with unreachable code is that it might reveal a bug.

Instead of if(something-that's-always-true), perhaps the programmer really meant if(something-else-that's-sometimes-false). In that case, you might have to fix your conditional. Or, the bug may actually be a "feature" now that the users have grown to appreciate and changing it could make them upset.

Or maybe they really meant to use the if(something-that's-always-true) but didn't realize it always evaluated to true, so they wrote the unreachable code "just in case". In that case, you probably just delete the unreachable code.

I'm sure there are other reasons for unreachable code.