Unreachable Code Detected (Why?)

Question

So, why is it that there is unreachable code detected in this code:

      public bool GetKeyPressed(KeyCode key)
    {
        for (int i = 0; i < keys.Count; i++)
            if (keys[i].key == key && keys[i].pressed)
                return true;
            else
                return false;

        return false;
    }

The index (i) is apparently unreachable ... why?

If your `keys` contains more than 1 key, it will return after the first iteration, and never find your desired key inside. Try removing the else in your for loop, e.g. `for (int i = 0; i < keys.Count; i++) if (keys[i].key == key && keys[i].pressed) return true;` only. — Keyur PATEL, Aug 26 '16 at 04:03
Just side note, why do you need loop if you are evaluating only first value? control will be returned to caller after first evaluation irrespective of key match. — Hari Prasad, Aug 26 '16 at 04:06

score 4 · Accepted Answer · answered Aug 26 '16 at 04:09

4

Your code has a loop that evaluates once, therefore the first iteration will always return.

If that's what you want, just say that,

return keys[0].key == key && keys[0].pressed;

If however (which is what I suspect here), you want to return true if any in the array meets your test, then use LINQ's Any(),

return keys.Any(k => k.key == key && k.pressed);

answered Aug 26 '16 at 04:09

jdphenix

15,022
3
41
74

holy cow, LINQ is beautiful. So... Just so I know what I'm doing here. The 'Any' is searching for 'Any' item in the list that contains the key I'm searching for and also makes sure it is pressed...? – Aug 26 '16 at 04:16
Basically - `enumerable.Any(condition)` is true if any elements meet the condition passed in. – jdphenix Aug 26 '16 at 15:21

sujith karivelil · Answer 2 · 2016-08-26T04:10:48.453

0

Your code having two code path, one is through if condition and another is through the else. Which means the control will leave the function in these two ways. So the return statement after else will never triggers. That is why the compiler points it as unreachable code. This can be avoided by using the following code.

public bool GetKeyPressed(KeyCode key)
{
    for (int i = 0; i < keys.Count; i++)
        if (keys[i].key == key && keys[i].pressed)
            return true;          
        return false;
}

edited Aug 26 '16 at 04:10

answered Aug 26 '16 at 04:07

sujith karivelil

28,671
6
55
88

"let me add some explanations;" --- why not add them first then post? – zerkms Aug 26 '16 at 04:10
@zerkms : I'm seeing that experts in this site, will add some key points initially and edit the post with complete explanations; just following the leaders. And some peoples will hit down for not explaining, but they wont remove the down after the edit, that's why I have added that quote. – sujith karivelil Aug 26 '16 at 04:12
1

@zerkms sometimes you've to kick off things first to win the race :-) – Hari Prasad Aug 26 '16 at 04:13
I probably would've gone for this approach if I wasn't shown the LINQ alternative. But thank you anyways :). – Aug 26 '16 at 04:21

score 0 · Answer 3 · edited May 23 '17 at 12:22

Expanding on jdphenix answer, I think we need to understand why your loop "evaluates once" and only then can we understand why there is "unreachable code".

public bool GetKeyPressed(Keys key)
{
    for (int i = 0; i < keys.Count; i++)
        if (keys[i].key == key && keys[i].pressed)
            return true;
        else
            return false;

    return false;
}

The for loop has a single if-else statement that makes up the body. The if guard if satisfied returns true otherwise it executes the next else statement which returns false. The net result is that at most a single cycle of the loop will be executed before control is returned to the calling method irrespective of the number of items in keys.

It's more obvious if we look at the code via JetBrains Resharper:

The code may as well be written as:

public bool GetKeyPressed(Keys key)
{
    for (int i = 0; i < keys.Count; ) // Look Ma, no i++  !!!
        if (keys[i].key == key && keys[i].pressed)
            return true;
        else
            return false;

    return false;
}

Don't make the mistake of thinking that the very last return false at the end of the method is not required because it is during the scenario where keys.Count == 0

Of course, formatting the code a bit more nicely goes a long way into revealing the problem that the first return false is redundant and could be simplified as per un-lucky's answer:

Unreachable Code Detected (Why?)

3 Answers3