Unreachable code detected

Unity Engine
1

This is pretty minor, but it kinda bugs me…

I’m getting compiling warnings for “unreachable code detected.” And yeah, it is currently unreachable, but they are just little bits that we are supposed to have for good practice.

My function returns a sprite for a world map avatar. I have unreachable code in the form of my “break” lines between cases. I also have a final failsafe/default return value at the end of the function that is deemed unreachable.

I feel a bit torn here. On one hand, I don’t want to get these warnings about unreachable code every time I compile (and neither do I want to disable these in case I make unreachable code somewhere else,) and on the other hand I feel like those bits should be left in, as good practice, in case I ever make revisions that don’t execute as cleanly as I want.

Sprite FindCurrentSprite(byte thisFrame)
    {

        switch (facingAngle)
        {

            case float n when n >= 45 && n < 135 || (n <= -225 && n > -315):
                if (thisFrame == 1)
                    return WalkingFrames.North1;
                else if (thisFrame == 3)
                    return WalkingFrames.North3;
                else
                    return WalkingFrames.North2;
                break;

            case float n when Mathf.Abs(n) >= 135 && Mathf.Abs(n) < 225:
                if (thisFrame == 1)
                    return WalkingFrames.West1;
                else if (thisFrame == 3)
                    return WalkingFrames.West3;
                else
                    return WalkingFrames.West2;
                break;

            default:
            case float n when (n >= 225 && n < 315) || (n <= -45 && n > -135):
                if (thisFrame == 1)
                    return WalkingFrames.South1;
                else if (thisFrame == 3)
                    return WalkingFrames.South3;
                else
                    return WalkingFrames.South2;
                break;

            case float n when Mathf.Abs(n) >= 315 || Mathf.Abs(n) < 45:
                if (thisFrame == 1)
                    return WalkingFrames.East1;
                else if (thisFrame == 3)
                    return WalkingFrames.East3;
                else
                    return WalkingFrames.East2;
                break;

        }

        return WalkingFrames.South2;
    }
2

I’d ignore the warning, or rewrite it as below. It should function the same as your version without the warnings.

Sprite FindCurrentSprite(byte thisFrame)
    {

        Sprite returnSprite = WalkingFrames.South2;
        switch (facingAngle)
        {

            case float n when n >= 45 && n < 135 || (n <= -225 && n > -315):
                if (thisFrame == 1)
                    returnSprite =  WalkingFrames.North1;
                else if (thisFrame == 3)
                    returnSprite = WalkingFrames.North3;
                else
                    returnSprite = WalkingFrames.North2;
                break;

            case float n when Mathf.Abs(n) >= 135 && Mathf.Abs(n) < 225:
                if (thisFrame == 1)
                    returnSprite = WalkingFrames.West1;
                else if (thisFrame == 3)
                    returnSprite = WalkingFrames.West3;
                else
                    returnSprite = WalkingFrames.West2;
                break;

            default:
            case float n when (n >= 225 && n < 315) || (n <= -45 && n > -135):
                if (thisFrame == 1)
                    returnSprite = WalkingFrames.South1;
                else if (thisFrame == 3)
                    returnSprite = WalkingFrames.South3;
                else
                    returnSprite = WalkingFrames.South2;
                break;

            case float n when Mathf.Abs(n) >= 315 || Mathf.Abs(n) < 45:
                if (thisFrame == 1)
                    returnSprite = WalkingFrames.East1;
                else if (thisFrame == 3)
                    returnSprite = WalkingFrames.East3;
                else
                    returnSprite = WalkingFrames.East2;
                break;

        }

        return returnSprite;
    }
1 Like
3

I think you should remove the bits that are unreachable, if in future you’ll forget to return a sprite somewhere you’ll get compiler error that not all code paths return a value. Same with "break"s, if your case won’t handle all the scenarios (that means no else clause) then the compiler will complain about missing break statement. So you should be covered on both fronts.

Also what is the purpose of that default in line 25 before two more cases?

4

@Joe-Censored Hmm, that’s a clever way to handle it.

Uhh, are you sure about that? If there are no “breaks” doesn’t that just mean the code is expected to drop through? Isn’t that a legal design?
Maybe I’m wrong but I thought it was.

The default result (in case it is ever sent garbage data) is identical to the “south” result. Rather than duplicate the code I just have it fall through.

5

Yeah I ran into this myself before, and generally never return from within a switch to avoid this annoyance. I like leaving the breaks in too, because I’ve commented out a return before while debugging and ended up with madness :stuck_out_tongue: . So now I just return after the switch like what I posted above.

6

In c# you can’t have fall through if the case statement is not empty, just try to remove both else statement and break, you should get compiler error

Ahh ok now I see it, I’m used to always putting default after the case so I didn’t get it at first :slight_smile:

7

If you really want to keep the return-from-middle, you could replace the switch with another cascading IF. Start if off with n=facingAngle to make it nearly identical.