Return value from For loop

Question

I have a listView in my app. I loop through the items to check which one is currently selected and then return a value. As all paths have to return a value I have to return a value outside the loop which overwrites the for loop return, how do I keep this without overwriting it after the loop?

public string GetItemValue()
{
    for (int i = 0; i < listView1.Items.Count; i++)
    {
        if (listView1.Items[i].Checked == true)
        {
            return listView1.Items[i].Text; // I want to keep this value
        }
     }
     // Without overwriting it with this but the compiler 
     // requires me to return a value here
     return "Error"; 
}

Any help is most appreciated. Thanks.

P.S I have tried using break after the if but no luck.

Answer 1

Actually, there is no need for a loop here:

return (listView1.SelectedItems.Count > 0)
    ? listView1.SelectedItems[0].Text
    : "Error";

But, as it was said, the original question is misleading since return does not override values. You might be thinking about assignment, instead of return. In this case a working code can look like this:

string ret = "Error";
foreach(var item in listView1.Items)
{
    if(item.Checked) { ret = item.Text; break; }
}

return ret;

Answer 2

On edit: bringing down my comment from above.

You don't need to worry about this. As soon as it hits the first return inside the loop, it will return immediately with that value. No code outside the loop is ever hit in that case.

Incidentally, this code would be cleaner:

public string GetItemValue()
{
    foreach (var item in listView1.Items)
    {
        if (item.Checked) return item.Text;
    }
    throw new InvalidOperationException("No checked items found");
}

Exceptions are a more idiomatic way of handling errors, and the foreach loop is preferred to a for loop when you're just iterating over collections.

Also using LINQ, you can get even more concise:

public string GetItemValue()
{
    return listView1.Items.Cast<ListViewItem>().Single(i => i.Checked).Text;
}

Answer 3

If you're returning a value in the loop, it should not reach the return that's outside of the loop. I would check to make sure that the loop is finding the selected item.

The other option is to create a local variable to hold the return value:

string returnValue = "Error";

for (int i = 0; i < listView1.Items.Count; i++) 
{ 
    if (listView1.Items[i].Checked == true) 
    { 
        returnValue = listView1.Items[i].Text;
        break;
    } 
} 

return returnValue;

Lastly, you might also consider returning an exception when no selections are found, and handling the exception from the calling method.

Answer 4

Well, your return outside of the loop, return "Error"; shouldn't get called based on your logic. Since return causes your method to immediately exit, the "Error" return will never happen, that is unless the code never steps into the if inside the loop.

All things considered, this may be an exceptional case in your code. Thus, throwing an exception may be the appropriate thing to do:

public string GetItemValue()
        {
            for (int i = 0; i < listView1.Items.Count; i++)
            {
                if (listView1.Items[i].Checked == true)
                {
                    return listView1.Items[i].Text; // I want to keep this value
                }
            }
            throw new InvalidOperationException("Did not find value expected.");
        }

Exceptions are usually thrown to indicate that a bug exists in code. A "Hey, that should really not be happening." The application stops, and hopefully the user gets an opportunity to contact support to help you reproduce it.

Based on your comment:

When I run it, it just returns the error text...

That means your check in your if statement is not succeeding.

if (listView1.Items[i].Checked == true)

Which means that none of your items in your ListView are checked.

Answer 5

The compiler requires that all paths of the function return a value. The compiler cannot know before hand whether your inner loop if condition will be met. You can cache the value at a variable and return this in the end of the function e.g. :

public string GetItemValue()
    {
        string temp = null;
        for (int i = 0; i < listView1.Items.Count; i++)
        {
            if (listView1.Items[i].Checked == true)
            {
                temp = listView1.Items[i].Text; // I want to keep this value
                break;
            }
        }
        return temp; // Without overwriting it with this but the compiler requires me to return a value here
    }

Answer 6

The return in your for loop isn't overwritten -- the method will return the value in the loop if your conditions are met. Execution of the method ends immediately upon reaching a return statement.

If your method is returning "Error", then I'd recommend looking at your code in a debugger, because it's reaching the end of the loop and returning the value "Error".

Answer 7

That's the kind of situations where you are probably better of throwing an exception in order to signal the exceptional situation:

public string GetItemValue()
{
    for (int i = 0; i < listView1.Items.Count; i++)
    {
        if (listView1.Items[i].Checked == true)
        {
            // Here you are leaving the GetItemValue method
            // and the loop stops
            return listView1.Items[i].Text;
        }
    }
    // if we get that far it means that none of the items of
    // the select list was actually checked => we are better of
    // reporting this to the caller of the method
    throw new Exception("Please select a value");
}

Answer 8

The return "Error" bit will not overwrite your loop return value. When a return is hit, the function exits, so when a value that is selected is found, the function will spit out your data and stop.