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

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.

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?
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.
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

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")

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

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.

Oh it is happening to the line that says b++ in the for loop,
@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

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

2 Comments

Related