-4

I wrote the following function, but I am getting an error that says "not all code paths return a value". I've looked this up online and searched through questions on stack overflow, but I just can see what I'm missing / doing wrong.

I am trying to use this function in an if statement elsewhere that checks if this function returns true or false. If true, it will do work. If false, it will return an error to the user.

private bool IsStatusChangeValid(CommandResult result)
{
    var file = FileViews.FileGet(fileId);

    // LOOP THAT CHECKS IF STATUS IS CHANGED TO "VOIDED"
    // THEN NOTIFIES USER IF THERE ARE NON-VOIDED ITEMS IN FILE
    foreach (var item in file.FileItems)
    {
        item.File = file;

        // ONLY IF ITEMS EXIST
        if (item.ItemCode.Length > 0)
        {
            // CHECK IF STATUS IS CHANGED TO "VOIDED"
            if (newDescription.Equals("Voided"))
            {
                if (item.ItemStatusID != ItemStatusIDConstants.Voided)
                {
                    result.Success = false;
                }
                return result.Success;
            }
        }
    }
}
AtlasBowler
  • 267
  • 1
  • 8
  • 18
  • you are not returning anything if (item.ItemCode.Length <= 0) – Val Nolav Apr 22 '15 at 14:41
  • 1
    You could get billions of answers just googling exception. It would be 10 times faster then adding question here – Giorgi Nakeuri Apr 22 '15 at 14:45
  • In my post: "I've looked this up online and searched through questions on stack overflow, but I just can see what I'm missing / doing wrong." – AtlasBowler Apr 22 '15 at 14:50
  • If you looked up anything your question would include text like "I've read help provided by VS when I click F1 on error message (https://msdn.microsoft.com/en-us/library/87cz4k9t%28v=vs.90%29.aspx) as well searching for error message itself https://www.bing.com/search?q=not+all+code+paths+return+a+value. I found http://stackoverflow.com/questions/21197410/c-sharp-returning-error-not-all-code-paths-return-a-value, but .... (i.e. name of function is different and I can't apply the advice)". Since there is no such text people assume you've searched on http://disney.com – Alexei Levenkov Apr 22 '15 at 15:04
  • I don't have to write it out that way. I stated that I was unable to see what I was missing or doing wrong based on what I looked up which is the same thing you said just a lot shorter. Assuming anything (especially online) is a terrible way to do things. You always end up sounding like a jerk when you could have just been helpful to begin with. – AtlasBowler Apr 22 '15 at 17:01

5 Answers5

2

After your foreach, it should return a value.

private bool IsStatusChangeValid(CommandResult result)
{
    var file = FileViews.FileGet(fileId);

    // LOOP THAT CHECKS IF STATUS IS CHANGED TO "VOIDED"
    // THEN NOTIFIES USER IF THERE ARE NON-VOIDED ITEMS IN FILE
    foreach (var item in file.FileItems)
    {
        item.File = file;

        // ONLY IF ITEMS EXIST
        if (item.ItemCode.Length > 0)
        {
            // CHECK IF STATUS IS CHANGED TO "VOIDED"
            if (newDescription.Equals("Voided"))
            {
                if (item.ItemStatusID != ItemStatusIDConstants.Voided)
                {
                    result.Success = false;
                }
            return result.Success;
            }
        }
    }

    return something here;
}
jvdveuten
  • 621
  • 4
  • 23
1

Yes, every code path needs to return a value. Otherwise it is not guaranteed that the method returns anything. This is what the compiler tries to tell you.

In your method it's possible that the if is never entered where you return the bool.

You can ensure that in different way, for example:

private bool IsStatusChangeValid(CommandResult result)
{
    bool isStatusChangeValid = true
    var file = FileViews.FileGet(fileId);

    foreach (var item in file.FileItems)
    {
        item.File = file;

        if (item.ItemCode.Length > 0)
        {
            if (newDescription.Equals("Voided"))
            {
                if (item.ItemStatusID != ItemStatusIDConstants.Voided)
                {
                    result.Success = false;
                }
                isStatusChangeValid = false
                break;
            }
        }
    }
    return isStatusChangeValid;
}
Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939
0

What if file.FileItems contained no items? What gets returned then?

What if none of the items have an ItemCode with Length > 0?

What if newDescription doesn't equal "Voided"?

At the very least, you need to add a return statement after the foreach loop. As to what is correct depends on your requirements.

Charles Mager
  • 25,735
  • 2
  • 35
  • 45
0

You need to return after the foreach loop for the case when it falls through the loop without it returning anything.

private bool IsStatusChangeValid(CommandResult result)
{
    var file = FileViews.FileGet(fileId);

    // LOOP THAT CHECKS IF STATUS IS CHANGED TO "VOIDED"
    // THEN NOTIFIES USER IF THERE ARE NON-VOIDED ITEMS IN FILE
    foreach (var item in file.FileItems)
    {
        item.File = file;

        // ONLY IF ITEMS EXIST
        if (item.ItemCode.Length > 0)
        {
            // CHECK IF STATUS IS CHANGED TO "VOIDED"
            if (newDescription.Equals("Voided"))
            {
                if (item.ItemStatusID != ItemStatusIDConstants.Voided)
                {
                    result.Success = false;
                    break;
                }
            }
        }
    }

    return result.Success;
}
Daniel Imms
  • 47,944
  • 19
  • 150
  • 166
0

Simply add

return false;

at the very end of your function

DrKoch
  • 9,556
  • 2
  • 34
  • 43