C#: Not all code paths return value and unreachable code?

Question

Hey so i have this code where i check to see if a player can remove an 'Item' from their inventory. The 'Inventory' is a Sorted Dictionary(Item, int) (subquestion: do i NEED a sorted dictionary to be able to access items in it with an index number??), and an Item is a class.

     public bool CanRemoveFromItemInventory(string item)
    {
        bool temp = false;
        if (ItemInventory.Count() <= 0)
        {
            return false;
        }
        else if (ItemInventory.Count() > 0)
        {
            for (int b = 0; b < ItemInventory.Count(); b++)
            {
                Item i = ItemInventory.Keys.ElementAt(b);
                if (i.GetName().Equals(item) && ItemInventory[i] >= 1)
                {
                    temp = true;
                }
                else
                {
                    temp = false;
                }

                if (!temp)
                {
                    return false;
                }
                else
                {
                    return true;
                }
            }
        }
        else
        {
            return temp;

         }
    }

The type of the object you want to store - if I understood you correctly, you were looking for a way to store objects in a collection where you could access them directly using their int code — sternr
– sternr, Commented Aug 6, 2011 at 22:41
well the int in the dictionary is their quantity, so basically i wanted to be able to access them by two methods, by name, or by position (in other words index) in the inventory — Kartik
– Kartik, Commented Aug 6, 2011 at 22:50

Marc Gravell · Accepted Answer · 2011-08-06 22:25:51Z

6

The compiler doesn't attempt to understand the logic - it just applies the rules. As far as it is concerned, it is possible that the for loop executes zero times, hence the middle block is missing a return value:

    else if (ItemInventory.Count() > 0)
    {
        for (int b = 0; b < ItemInventory.Count(); b++)
        {
              // ... this always returns something
        }
        // BUT STILL NEED TO EITHER RETURN OR THROW HERE
    }

indeed, it is correct in this - as an evil malcontent could write a Count() method that returns different values each call (or to present a less evil scenario - a thread race / data mutation).

Perhaps the easiest "fix" here is to change:

    else
    {
        return temp;
    }

to simply:

    return temp;

then it will apply to all the branches.

answered Aug 6, 2011 at 22:25

Marc Gravell

1.1m273 gold badges2.6k silver badges3k bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Kartik Over a year ago

ohhh i see! So what would you suggest I do to fix this error? I did put the if then statement to prevent the for loop from even happening, but i see what you mean. Is some piece of code that would handle that event? Like an 'else' code block for the 'for' loop?

Marc Gravell Over a year ago

@Kartik refresh for my suggestion

Kartik Over a year ago

So here is what I did... The only problem I am foreseeing is that it will go through all the Items in the Inventory and the value of temp will change over and over again, until it is done with the whole Inventory. Basically it will only return true only if the user tries to remove the last item of the inventory.

Kartik Over a year ago

You know? Never mind! I figured it out! I just set 'b' to to the total number of items in the inventory if the item DOES exist in the inventory... and i added what you wrote here. Thank you so much!!

Michael Stum · Accepted Answer · 2011-08-06 22:28:39Z

2

Put the return outside of the for loop:

for (int b = 0; b < ItemInventory.Count(); b++) {
     Item i = ItemInventory.Keys.ElementAt(b);
     if (i.GetName().Equals(item) && ItemInventory[i] >= 1) {
         temp = true;
         break;
     }
}
return temp;

Reasoning: If Count() is 0, the for loop would never be executed and thus you would never raturn. While this maybe unlikely (after all, you check that Count is bigger than 0 before). the compiler is unable to determine that, because Count() can be modified between the if-check and the for loop. The compiler is not able to determine this (see the "Halting Problem")

answered Aug 6, 2011 at 22:28

Michael Stum

182k120 gold badges411 silver badges541 bronze badges

Comments

cdhowie · Accepted Answer · 2011-08-07 01:06:21Z

2

The compiler is noticing that there is no return statement after the for loop. The compiler cannot prove that the code path will never leave (or even enter) the for loop, and so it is expecting to hit a return statement sometime after the loop. However, you have no return statement on that path.

As for the unreachable code error, you will need to add a comment on the line it's complaining about.

Update: It looks like you're going for something like this:

public bool CanRemoveFromItemInventory(string item)
{
    var pair = ItemInventory.FirstOrDefault(pair => pair.Key.GetName() == item);

    return pair != null && pair.Value >= 1;
}

The current version of your code will fail if the first item in the inventory list is not the item you are searching for. In other words, even if you fix the compiler errors, your code will still not function correctly.

edited Aug 7, 2011 at 1:06

answered Aug 6, 2011 at 22:27

cdhowie

172k25 gold badges303 silver badges324 bronze badges

2 Comments

Kartik Over a year ago

Oh it is happening to the line that says b++ in the for loop,

cdhowie Over a year ago

@Kartik: That's because there is a return statement at the end of the for loop, so the loop is actually only a conditional -- the loop body will never run more than once, because you always return. I am going to update my answer.

Collectives™ on Stack Overflow

C#: Not all code paths return value and unreachable code?

3 Answers 3

4 Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related