C# error - "Not all code paths return a value"

Question

This is fairly simple method. I use entity framework to get some data and then check some values in a if statement. However right now the method is marked with red.

This is my method:

private bool IsSoleInProduction(long? shoeLastID) { if (shoeLastID == null) { MessageBox.Show(Resources.ERROR_SAVE, "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); return false; } ISoleService soleService = UnityDependencyResolver.Instance.GetService<ISoleService>(); List<Sole> entity = soleService.All().Where(s => s.ShoeLastID == shoeLastID).ToList(); if (entity.Count() != 0) { foreach (var items in entity) { if (items.Status == 20) { return true; } else { return false; } } } else { return false; } }

What am I missing?

Did you modify the code after the answers? The above code should work. — Emil Lundin
– Emil Lundin, Commented Apr 9, 2013 at 7:06
wouldn't the if else status check only ever check the first element of the list? I'd think you just need the one return false afterwards — Sayse
– Sayse, Commented Apr 9, 2013 at 7:10

cuongle · Accepted Answer · 2013-04-09 07:29:43Z

7

You need to take advantage of LINQ with Any, replace your code:

if (entity.Count() != 0) { foreach (var items in entity) { if (items.Status == 20) { return true; } else { return false; } } } else { return false; }

with simpler code:

 return entity.Any(item => item.Status == 20);

Or even better performance:

 return soleService.All() .Any(s => s.ShoeLastID == shoeLastID && s.Status == 20);

Edit: With you comment, below code is what you need:

 List<Sole> entity = soleService.All() .FirstOrDefault(s => s.ShoeLastID == shoeLastID); return entity == null ? false : entity.Status == 20;

edited Apr 9, 2013 at 7:29

answered Apr 9, 2013 at 7:09

cuongle

75.5k30 gold badges155 silver badges212 bronze badges

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

6 Comments

josef.axa Over a year ago

The version using linq is not equivalent to the original, which may return false if there are items with Status != 20 at the beginning of the list.

cuongle Over a year ago

@josef.axa: No, it returns if one item which has Status == 20, if not, will search until found. You need to run this code to see

Emil Lundin Over a year ago

@josef.axa That is a good abservation. The equivalent version would be something like: return (soleService.All().First(s => s.ShoeLastID == shoeLastID).Status == 20)

cuongle Over a year ago

@josef.axa: I have included the link on this answer

josef.axa Over a year ago

What I meant is the linq version will return true if the list contains any element with status == 20, while the original will only return true if the first item has status == 20.

|

JustAnotherUserYouMayKnow · Accepted Answer · 2013-04-09 07:01:54Z

If there's no item in your entity collection, then neither of the containing if/else branches will be executed. In this case there's no return statement anymore, because the else part won't be executed, and outside your foreach you have no return statement.

Jeppe Stig Nielsen · Accepted Answer · 2013-04-09 07:15:44Z

The compiler does not "see" that if

entity.Count() != 0

then your loop

foreach (var items in entity)

will run at least once. Therefore it sees a possibility of running the forech zero times, and not running the else block.

Suppose first time the entity is enumerated, it yields some (finite number of) items. Then the Count will be non-zero. Then suppose next time the same entity is enumerated then it yields no items! That would cause your code to "fall through" without returning.

It is very probably that you can guarantee that the source yields the same number of items each time it is re-enumerated. But the compiler cannot.

Solution: Just skip if (entity.Count() != 0) and do foreach right away.

Sachin · Accepted Answer · 2013-04-09 07:02:41Z

You haven't retrun anything from this code block

if (entity.Count() != 0) { foreach (var items in entity) { if (items.Status == 20) { return true; } else { return false; } } // return someting }

Community · Accepted Answer · 2017-05-23 11:49:58Z

You might consider doing the following. This will adhere to the "single exit-point" principle (which can sometimes help improve code clarity), and ensure you have a default value in any case:

private bool IsSoleInProduction(long? shoeLastID) { // The main change: A default value, assuming "no": var isSoleInProduction = false; if (shoeLastID == null) { MessageBox.Show(Resources.ERROR_SAVE, "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); isSoleInProduction = false; } ISoleService soleService = UnityDependencyResolver.Instance.GetService<ISoleService>(); List<Sole> entity = soleService.All().Where(s => s.ShoeLastID == shoeLastID).ToList(); if (entity.Count() != 0) { foreach (var items in entity) { if (items.Status == 20) { isSoleInProduction = true; } else { isSoleInProduction = false; } } } else { isSoleInProduction = false; } return isSoleInProduction; }

Soner Gönül · Accepted Answer · 2013-04-09 07:15:47Z

What will be your entity.Count() is not 0 and your entity doesn't have any items?

That means your if block will work but foreach part will not work. Since your if part doesn't have any return statement, that's why you get an error.

You should put return statement in your if part.

if (entity.Count() != 0) { foreach (var items in entity) { if (items.Status == 20) { return true; } else { return false; } } //return true or false }

Rune Andersen · Accepted Answer · 2013-04-09 07:04:27Z

If your entity collection has no elements you will not reach a return statement - you need to add a return false forexample as last statement

mamoo · Accepted Answer · 2013-04-09 07:05:51Z

As the error states there can be cases in which none of your return clause is evaluated (e.g. if there are no elements in your list). To quickly solve it you can put a default return statement, for example by moving the last return clause outside the else statement. But it really depends on the behavior you'd expect.

 private bool IsSoleInProduction(long? shoeLastID) { if (shoeLastID == null) { MessageBox.Show(Resources.ERROR_SAVE, "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); return false; } ISoleService soleService = UnityDependencyResolver.Instance.GetService<ISoleService>(); List<Sole> entity = soleService.All().Where(s => s.ShoeLastID == shoeLastID).ToList(); if (entity.Count() != 0) { foreach (var items in entity) { if (items.Status == 20) { return true; } else { return false; } } } return false; }

Matthew Strawbridge · Accepted Answer · 2013-04-09 07:19:50Z

The compiler can't guarantee that the first call to Count means that the foreach will loop at least once (with good reason, because you could, if you wished, create a collection where this wasn't true). You can do this instead (with no need for the outer if):

foreach (var items in entity) { if (items.Status == 20) { return true; } else { return false; } } return false;

Collectives™ on Stack Overflow

C# error - "Not all code paths return a value"

9 Answers 9

6 Comments

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

9 Answers 9

6 Comments

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Linked

Related