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?

Darin Dimitrov · Accepted Answer · 2010-11-13T14:00:08.313

3

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.

edited Nov 13 '10 at 14:00

answered Nov 13 '10 at 13:55

Darin Dimitrov

1,023,142
271
3,287
2,928

Thanks @Darin for your reply. That makes sense alright. Do you think it would be okay to go ahead with the deployment with this warning so? – 109221793 Nov 13 '10 at 13:57
2

With this specific case it is OK. But from readability perspective and maintenance of your code I wouldn't leave it. So if you have access to the source code then modify it. It won't take much of a time and it will remove unnecessary code and the less code you have the less things to worry about :-) – Darin Dimitrov Nov 13 '10 at 13:59

score 1 · Answer 2 · answered Nov 13 '10 at 14:00

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.

score 0 · Answer 3 · answered Nov 13 '10 at 14:20

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.

Unreachable Code Detected

3 Answers3